Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up special function const checks #90273

Merged
merged 2 commits into from
Oct 27, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 20 additions & 32 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,34 +30,25 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> {
&mut self,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx>],
is_const_fn: bool,
) -> InterpResult<'tcx, Option<ty::Instance<'tcx>>> {
// The list of functions we handle here must be in sync with
// `is_lang_special_const_fn` in `transform/check_consts/mod.rs`.
// All `#[rustc_do_not_const_check]` functions should be hooked here.
let def_id = instance.def_id();

if is_const_fn {
if Some(def_id) == self.tcx.lang_items().const_eval_select() {
// redirect to const_eval_select_ct
if let Some(const_eval_select) = self.tcx.lang_items().const_eval_select_ct() {
return Ok(Some(
ty::Instance::resolve(
*self.tcx,
ty::ParamEnv::reveal_all(),
const_eval_select,
instance.substs,
)
.unwrap()
.unwrap(),
));
}
if Some(def_id) == self.tcx.lang_items().const_eval_select() {
// redirect to const_eval_select_ct
if let Some(const_eval_select) = self.tcx.lang_items().const_eval_select_ct() {
return Ok(Some(
ty::Instance::resolve(
*self.tcx,
ty::ParamEnv::reveal_all(),
const_eval_select,
instance.substs,
)
.unwrap()
.unwrap(),
));
}
return Ok(None);
}

if Some(def_id) == self.tcx.lang_items().panic_fn()
|| Some(def_id) == self.tcx.lang_items().panic_str()
|| Some(def_id) == self.tcx.lang_items().panic_display()
} else if Some(def_id) == self.tcx.lang_items().panic_display()
|| Some(def_id) == self.tcx.lang_items().begin_panic_fn()
{
// &str or &&str
Expand Down Expand Up @@ -286,19 +277,16 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
}
}

// Some functions we support even if they are non-const -- but avoid testing
// that for const fn!
// `const_eval_select` is a const fn because it must use const trait bounds.
if let Some(new_instance) = ecx.hook_special_const_fn(instance, args, is_const_fn)? {
// We call another const fn instead.
return Self::find_mir_or_eval_fn(ecx, new_instance, _abi, args, _ret, _unwind);
}

if !is_const_fn {
// We certainly do *not* want to actually call the fn
// though, so be sure we return here.
throw_unsup_format!("calling non-const function `{}`", instance)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the is_const_fn local and put this inside the block that sets it to true, since all the hooked fns are now const, we don't need to pass this as a parameter.

}

if let Some(new_instance) = ecx.hook_special_const_fn(instance, args)? {
// We call another const fn instead.
return Self::find_mir_or_eval_fn(ecx, new_instance, _abi, args, _ret, _unwind);
}
}
// This is a const fn. Call it.
Ok(Some(ecx.load_mir(instance.def, None)?))
Expand Down
42 changes: 19 additions & 23 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use std::ops::Deref;
use super::ops::{self, NonConstOp, Status};
use super::qualifs::{self, CustomEq, HasMutInterior, NeedsDrop, NeedsNonConstDrop};
use super::resolver::FlowSensitiveAnalysis;
use super::{is_lang_panic_fn, is_lang_special_const_fn, ConstCx, Qualif};
use super::{ConstCx, Qualif};
use crate::const_eval::is_unstable_const_fn;

// We are using `MaybeMutBorrowedLocals` as a proxy for whether an item may have been mutated
Expand Down Expand Up @@ -918,31 +918,27 @@ impl Visitor<'tcx> for Checker<'mir, 'tcx> {
}

// At this point, we are calling a function, `callee`, whose `DefId` is known...
if is_lang_special_const_fn(tcx, callee) {
// `begin_panic` and `panic_display` are generic functions that accept
// types other than str. Check to enforce that only str can be used in
// const-eval.

// const-eval of the `begin_panic` fn assumes the argument is `&str`
if Some(callee) == tcx.lang_items().begin_panic_fn() {
match args[0].ty(&self.ccx.body.local_decls, tcx).kind() {
ty::Ref(_, ty, _) if ty.is_str() => (),
_ => self.check_op(ops::PanicNonStr),
}
}

// const-eval of the `panic_display` fn assumes the argument is `&&str`
if Some(callee) == tcx.lang_items().panic_display() {
match args[0].ty(&self.ccx.body.local_decls, tcx).kind() {
ty::Ref(_, ty, _) if matches!(ty.kind(), ty::Ref(_, ty, _) if ty.is_str()) =>
{}
_ => self.check_op(ops::PanicNonStr),
}
// `begin_panic` and `panic_display` are generic functions that accept
// types other than str. Check to enforce that only str can be used in
// const-eval.

// const-eval of the `begin_panic` fn assumes the argument is `&str`
if Some(callee) == tcx.lang_items().begin_panic_fn() {
match args[0].ty(&self.ccx.body.local_decls, tcx).kind() {
ty::Ref(_, ty, _) if ty.is_str() => return,
_ => self.check_op(ops::PanicNonStr),
}
}

if is_lang_panic_fn(tcx, callee) {
// run stability check on non-panic special const fns.
return;
// const-eval of the `panic_display` fn assumes the argument is `&&str`
if Some(callee) == tcx.lang_items().panic_display() {
match args[0].ty(&self.ccx.body.local_decls, tcx).kind() {
ty::Ref(_, ty, _) if matches!(ty.kind(), ty::Ref(_, ty, _) if ty.is_str()) =>
{
return;
}
_ => self.check_op(ops::PanicNonStr),
}
}

Expand Down
18 changes: 0 additions & 18 deletions compiler/rustc_const_eval/src/transform/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,24 +72,6 @@ impl ConstCx<'mir, 'tcx> {
}
}

/// Returns `true` if this `DefId` points to one of the official `panic` lang items.
pub fn is_lang_panic_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool {
Some(def_id) == tcx.lang_items().panic_fn()
|| Some(def_id) == tcx.lang_items().panic_str()
|| Some(def_id) == tcx.lang_items().panic_display()
|| Some(def_id) == tcx.lang_items().begin_panic_fn()
|| Some(def_id) == tcx.lang_items().panic_fmt()
}

/// Returns `true` if this `DefId` points to one of the lang items that will be handled differently
/// in const_eval.
pub fn is_lang_special_const_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool {
// We can allow calls to these functions because `hook_special_const_fn` in
// `const_eval/machine.rs` ensures the calls are handled specially.
// Keep in sync with what that function handles!
is_lang_panic_fn(tcx, def_id) || Some(def_id) == tcx.lang_items().const_eval_select()
}

pub fn rustc_allow_const_fn_unstable(
tcx: TyCtxt<'tcx>,
def_id: DefId,
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_const_eval/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use rustc_index::vec::{Idx, IndexVec};
use std::cell::Cell;
use std::{cmp, iter, mem};

use crate::transform::check_consts::{is_lang_special_const_fn, qualifs, ConstCx};
use crate::transform::check_consts::{qualifs, ConstCx};
use crate::transform::MirPass;

/// A `MirPass` for promotion.
Expand Down Expand Up @@ -656,9 +656,7 @@ impl<'tcx> Validator<'_, 'tcx> {
}

let is_const_fn = match *fn_ty.kind() {
ty::FnDef(def_id, _) => {
self.tcx.is_const_fn_raw(def_id) || is_lang_special_const_fn(self.tcx, def_id)
}
ty::FnDef(def_id, _) => self.tcx.is_const_fn_raw(def_id),
_ => false,
};
if !is_const_fn {
Expand Down
1 change: 1 addition & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
#![feature(const_discriminant)]
#![feature(const_float_bits_conv)]
#![feature(const_float_classify)]
#![feature(const_fmt_arguments_new)]
#![feature(const_heap)]
#![feature(const_inherent_unchecked_arith)]
#![feature(const_int_unchecked_arith)]
Expand Down
19 changes: 9 additions & 10 deletions library/core/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,10 @@ use crate::panic::{Location, PanicInfo};
// never inline unless panic_immediate_abort to avoid code
// bloat at the call sites as much as possible
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller]
#[lang = "panic"] // needed by codegen for panic on overflow and other `Assert` MIR terminators
pub fn panic(expr: &'static str) -> ! {
if cfg!(feature = "panic_immediate_abort") {
super::intrinsics::abort()
}

pub const fn panic(expr: &'static str) -> ! {
// Use Arguments::new_v1 instead of format_args!("{}", expr) to potentially
// reduce size overhead. The format_args! macro uses str's Display trait to
// write expr, which calls Formatter::pad, which must accommodate string
Expand All @@ -52,15 +49,16 @@ pub fn panic(expr: &'static str) -> ! {

#[inline]
#[track_caller]
#[lang = "panic_str"] // needed for const-evaluated panics
pub fn panic_str(expr: &str) -> ! {
panic_fmt(format_args!("{}", expr));
#[lang = "panic_str"] // needed for `non-fmt-panics` lint
pub const fn panic_str(expr: &str) -> ! {
panic_display(&expr);
}

#[inline]
#[track_caller]
#[lang = "panic_display"] // needed for const-evaluated panics
pub fn panic_display<T: fmt::Display>(x: &T) -> ! {
#[rustc_do_not_const_check] // hooked by const-eval
pub const fn panic_display<T: fmt::Display>(x: &T) -> ! {
panic_fmt(format_args!("{}", *x));
}

Expand Down Expand Up @@ -89,7 +87,8 @@ fn panic_bounds_check(index: usize, len: usize) -> ! {
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller]
#[lang = "panic_fmt"] // needed for const-evaluated panics
pub fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! {
#[rustc_do_not_const_check] // hooked by const-eval
pub const fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! {
if cfg!(feature = "panic_immediate_abort") {
super::intrinsics::abort()
}
Expand Down
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@
#![feature(const_cstr_unchecked)]
#![feature(const_fn_floating_point_arithmetic)]
#![feature(const_fn_fn_ptr_basics)]
#![feature(const_fn_trait_bound)]
#![feature(const_format_args)]
#![feature(const_io_structs)]
#![feature(const_ip)]
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,8 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cold]
#[track_caller]
pub fn begin_panic<M: Any + Send>(msg: M) -> ! {
#[rustc_do_not_const_check] // hooked by const-eval
pub const fn begin_panic<M: Any + Send>(msg: M) -> ! {
if cfg!(feature = "panic_immediate_abort") {
intrinsics::abort()
}
Expand Down