about summary refs log tree commit diff
path: root/compiler
diff options
context:
space:
mode:
authorRalf Jung <post@ralfj.de>2024-03-21 13:53:00 +0100
committerRalf Jung <post@ralfj.de>2024-04-23 23:02:54 +0200
commit173d1bd36be1925c0cb0b0bf55740c26db4ac476 (patch)
treea4ecdef1b76bd546deb5d3346ad378310c62868b /compiler
parentbf021ea6258bcd886fae523e8448f3697dd041fe (diff)
downloadrust-173d1bd36be1925c0cb0b0bf55740c26db4ac476.tar.gz
rust-173d1bd36be1925c0cb0b0bf55740c26db4ac476.zip
properly fill a promoted's required_consts
then we can also make all_required_consts_are_checked a constant instead of a function
Diffstat (limited to 'compiler')
-rw-r--r--compiler/rustc_const_eval/src/const_eval/dummy_machine.rs7
-rw-r--r--compiler/rustc_const_eval/src/const_eval/machine.rs14
-rw-r--r--compiler/rustc_const_eval/src/interpret/eval_context.rs4
-rw-r--r--compiler/rustc_const_eval/src/interpret/machine.rs2
-rw-r--r--compiler/rustc_middle/src/mir/consts.rs15
-rw-r--r--compiler/rustc_mir_transform/src/promote_consts.rs14
-rw-r--r--compiler/rustc_mir_transform/src/required_consts.rs19
7 files changed, 30 insertions, 45 deletions
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 de8de021b09..7b6828c6e18 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 2d06460f353..dd835279df3 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 8f7c2580ad4..126d64329f8 100644
--- a/compiler/rustc_const_eval/src/interpret/eval_context.rs
+++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs
@@ -1179,9 +1179,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 e8ecbe089c3..8bc569bed54 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 596271c1e47..3c213914fc0 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<Scalar> {
         match self {
diff --git a/compiler/rustc_mir_transform/src/promote_consts.rs b/compiler/rustc_mir_transform/src/promote_consts.rs
index 928a23006fa..689a547689a 100644
--- a/compiler/rustc_mir_transform/src/promote_consts.rs
+++ b/compiler/rustc_mir_transform/src/promote_consts.rs
@@ -951,6 +951,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>(
@@ -995,11 +1003,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 {
@@ -1012,6 +1015,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 f0c63a6648c..71ac929d35e 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<ConstOperand<'tcx>>,
@@ -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);
         }
     }