diff --git a/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs b/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs index de8de021b0972..7b6828c6e1828 100644 --- a/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs @@ -46,11 +46,8 @@ impl<'mir, 'tcx: 'mir> interpret::Machine<'mir, 'tcx> for DummyMachine { type MemoryKind = !; const PANIC_ON_ALLOC_FAIL: bool = true; - #[inline(always)] - fn all_required_consts_are_checked(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { - // We want to just eval random consts in the program, so `eval_mir_const` can fail. - false - } + // We want to just eval random consts in the program, so `eval_mir_const` can fail. + const ALL_CONSTS_ARE_PRECHECKED: bool = false; #[inline(always)] fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 2d06460f35302..dd835279df331 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -375,20 +375,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, const PANIC_ON_ALLOC_FAIL: bool = false; // will be raised as a proper error - #[inline] - fn all_required_consts_are_checked(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { - // Generally we expect required_consts to be properly filled, except for promoteds where - // storing these consts shows up negatively in benchmarks. A promoted can only be relevant - // if its parent MIR is relevant, and the consts in the promoted will be in the parent's - // `required_consts`, so we are still sure to catch any const-eval bugs, just a bit less - // directly. - if ecx.frame_idx() == 0 && ecx.frame().body.source.promoted.is_some() { - false - } else { - true - } - } - #[inline(always)] fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { matches!(ecx.machine.check_alignment, CheckAlignment::Error) diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 7b14bb714584d..2661ceda210d6 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -1177,9 +1177,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> { M::eval_mir_constant(self, *val, span, layout, |ecx, val, span, layout| { let const_val = val.eval(*ecx.tcx, ecx.param_env, span).map_err(|err| { - if M::all_required_consts_are_checked(self) - && !matches!(err, ErrorHandled::TooGeneric(..)) - { + if M::ALL_CONSTS_ARE_PRECHECKED && !matches!(err, ErrorHandled::TooGeneric(..)) { // Looks like the const is not captued by `required_consts`, that's bad. bug!("interpret const eval failure of {val:?} which is not in required_consts"); } diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index a23190dd2d89c..30fb747915259 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -142,7 +142,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized { /// Determines whether `eval_mir_constant` can never fail because all required consts have /// already been checked before. - fn all_required_consts_are_checked(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool; + const ALL_CONSTS_ARE_PRECHECKED: bool = true; /// Whether memory accesses should be alignment-checked. fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool; diff --git a/compiler/rustc_middle/src/mir/consts.rs b/compiler/rustc_middle/src/mir/consts.rs index 02185cbeacf9e..b01ccecd4846f 100644 --- a/compiler/rustc_middle/src/mir/consts.rs +++ b/compiler/rustc_middle/src/mir/consts.rs @@ -238,6 +238,21 @@ impl<'tcx> Const<'tcx> { } } + /// Determines whether we need to add this const to `required_consts`. This is the case if and + /// only if evaluating it may error. + #[inline] + pub fn is_required_const(&self) -> bool { + match self { + Const::Ty(c) => match c.kind() { + ty::ConstKind::Value(_) => false, // already a value, cannot error + ty::ConstKind::Param(_) | ty::ConstKind::Error(_) => true, // these are errors or could be replaced by errors + _ => bug!("is_required_const: unexpected ty::ConstKind {:#?}", c), + }, + Const::Unevaluated(..) => true, + Const::Val(..) => false, // already a value, cannot error + } + } + #[inline] pub fn try_to_scalar(self) -> Option { match self { diff --git a/compiler/rustc_mir_transform/src/promote_consts.rs b/compiler/rustc_mir_transform/src/promote_consts.rs index fd50251f81696..e65302244dacc 100644 --- a/compiler/rustc_mir_transform/src/promote_consts.rs +++ b/compiler/rustc_mir_transform/src/promote_consts.rs @@ -950,6 +950,14 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Promoter<'a, 'tcx> { *local = self.promote_temp(*local); } } + + fn visit_constant(&mut self, constant: &mut ConstOperand<'tcx>, _location: Location) { + if constant.const_.is_required_const() { + self.promoted.required_consts.push(*constant); + } + + // Skipping `super_constant` as the visitor is otherwise only looking for locals. + } } fn promote_candidates<'tcx>( @@ -994,11 +1002,6 @@ fn promote_candidates<'tcx>( None, body.tainted_by_errors, ); - // We keep `required_consts` of the new MIR body empty. All consts mentioned here have - // already been added to the parent MIR's `required_consts` (that is computed before - // promotion), and no matter where this promoted const ends up, our parent MIR must be - // somewhere in the reachable dependency chain so we can rely on its required consts being - // evaluated. promoted.phase = MirPhase::Analysis(AnalysisPhase::Initial); let promoter = Promoter { @@ -1011,6 +1014,7 @@ fn promote_candidates<'tcx>( add_to_required: false, }; + // `required_consts` of the promoted itself gets filled while building the MIR body. let mut promoted = promoter.promote_candidate(candidate, promotions.len()); promoted.source.promoted = Some(promotions.next_index()); promotions.push(promoted); diff --git a/compiler/rustc_mir_transform/src/required_consts.rs b/compiler/rustc_mir_transform/src/required_consts.rs index f0c63a6648c6a..71ac929d35ebd 100644 --- a/compiler/rustc_mir_transform/src/required_consts.rs +++ b/compiler/rustc_mir_transform/src/required_consts.rs @@ -1,6 +1,5 @@ use rustc_middle::mir::visit::Visitor; -use rustc_middle::mir::{Const, ConstOperand, Location}; -use rustc_middle::ty; +use rustc_middle::mir::{ConstOperand, Location}; pub struct RequiredConstsVisitor<'a, 'tcx> { required_consts: &'a mut Vec>, @@ -14,21 +13,7 @@ impl<'a, 'tcx> RequiredConstsVisitor<'a, 'tcx> { impl<'tcx> Visitor<'tcx> for RequiredConstsVisitor<'_, 'tcx> { fn visit_constant(&mut self, constant: &ConstOperand<'tcx>, _: Location) { - // Only unevaluated consts have to be added to `required_consts` as only those can possibly - // still have latent const-eval errors. - let is_required = match constant.const_ { - Const::Ty(c) => match c.kind() { - ty::ConstKind::Value(_) => false, // already a value, cannot error - ty::ConstKind::Param(_) | ty::ConstKind::Error(_) => true, // these are errors or could be replaced by errors - _ => bug!( - "only ConstKind::Param/Value/Error should be encountered here, got {:#?}", - c - ), - }, - Const::Unevaluated(..) => true, - Const::Val(..) => false, // already a value, cannot error - }; - if is_required { + if constant.const_.is_required_const() { self.required_consts.push(*constant); } } diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 2ef589602c006..29315c4933cdd 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -871,12 +871,6 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { const PANIC_ON_ALLOC_FAIL: bool = false; - #[inline(always)] - fn all_required_consts_are_checked(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { - // We want to catch any cases of missing required_consts - true - } - #[inline(always)] fn enforce_alignment(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool { ecx.machine.check_alignment != AlignmentCheck::None diff --git a/tests/ui/consts/required-consts/collect-in-promoted-const.noopt.stderr b/tests/ui/consts/required-consts/collect-in-promoted-const.noopt.stderr new file mode 100644 index 0000000000000..784ef73e9bb7c --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-promoted-const.noopt.stderr @@ -0,0 +1,37 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-promoted-const.rs:9:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-promoted-const.rs:9:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-promoted-const.rs:20:21 + | +LL | let _val = &Fail::::C; + | ^^^^^^^^^^^^ + +note: erroneous constant encountered + --> $DIR/collect-in-promoted-const.rs:20:20 + | +LL | let _val = &Fail::::C; + | ^^^^^^^^^^^^^ + +note: erroneous constant encountered + --> $DIR/collect-in-promoted-const.rs:20:21 + | +LL | let _val = &Fail::::C; + | ^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +note: the above error was encountered while instantiating `fn f::` + --> $DIR/collect-in-promoted-const.rs:25:5 + | +LL | f::(); + | ^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-promoted-const.opt.stderr b/tests/ui/consts/required-consts/collect-in-promoted-const.opt.stderr new file mode 100644 index 0000000000000..4b6f84de83afb --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-promoted-const.opt.stderr @@ -0,0 +1,53 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-promoted-const.rs:9:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-promoted-const.rs:9:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-promoted-const.rs:20:21 + | +LL | let _val = &Fail::::C; + | ^^^^^^^^^^^^ + +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-promoted-const.rs:9:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-promoted-const.rs:9:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-promoted-const.rs:20:21 + | +LL | let _val = &Fail::::C; + | ^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +note: erroneous constant encountered + --> $DIR/collect-in-promoted-const.rs:20:20 + | +LL | let _val = &Fail::::C; + | ^^^^^^^^^^^^^ + +note: erroneous constant encountered + --> $DIR/collect-in-promoted-const.rs:20:21 + | +LL | let _val = &Fail::::C; + | ^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +note: the above error was encountered while instantiating `fn f::` + --> $DIR/collect-in-promoted-const.rs:25:5 + | +LL | f::(); + | ^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-promoted-const.rs b/tests/ui/consts/required-consts/collect-in-promoted-const.rs new file mode 100644 index 0000000000000..4a3ce92e8f90d --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-promoted-const.rs @@ -0,0 +1,26 @@ +//@revisions: noopt opt +//@ build-fail +//@[noopt] compile-flags: -Copt-level=0 +//@[opt] compile-flags: -O +//! Make sure we error on erroneous consts even if they get promoted. + +struct Fail(T); +impl Fail { + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed + //[opt]~^ ERROR evaluation of `Fail::::C` failed + // (Not sure why optimizations lead to this being emitted twice, but as long as compilation + // fails either way it's fine.) +} + +#[inline(never)] +fn f() { + if false { + // If promotion moved `C` from our required_consts to its own, without adding itself to + // our required_consts, then we'd miss the const-eval failure here. + let _val = &Fail::::C; + } +} + +fn main() { + f::(); +} diff --git a/tests/ui/consts/required-consts/interpret-in-promoted.rs b/tests/ui/consts/required-consts/interpret-in-promoted.rs index 187494180ad1d..48caece6ff56f 100644 --- a/tests/ui/consts/required-consts/interpret-in-promoted.rs +++ b/tests/ui/consts/required-consts/interpret-in-promoted.rs @@ -1,7 +1,7 @@ //@revisions: noopt opt //@[noopt] compile-flags: -Copt-level=0 //@[opt] compile-flags: -O -//! Make sure we error on erroneous consts even if they are unused. +//! Make sure we evaluate const fn calls even if they get promoted and their result ignored. const unsafe fn ub() { std::hint::unreachable_unchecked(); diff --git a/tests/ui/rfcs/rfc-1623-static/rfc1623-2.rs b/tests/ui/rfcs/rfc-1623-static/rfc1623-2.rs index f50b72f927942..97fc1276f6175 100644 --- a/tests/ui/rfcs/rfc-1623-static/rfc1623-2.rs +++ b/tests/ui/rfcs/rfc-1623-static/rfc1623-2.rs @@ -26,8 +26,8 @@ static SOME_STRUCT: &SomeStruct = &SomeStruct { foo: &Foo { bools: &[false, true] }, bar: &Bar { bools: &[true, true] }, f: &id, - //~^ ERROR mismatched types - //~| ERROR mismatched types + //~^ ERROR implementation of `FnOnce` is not general enough + //~| ERROR implementation of `FnOnce` is not general enough //~| ERROR implementation of `Fn` is not general enough //~| ERROR implementation of `Fn` is not general enough };