MIR required_consts, mentioned_items: ensure we do not forget to fill these lists

Author: RalfJung

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -888,7 +888,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         body: &'tcx mir::Body<'tcx>,
     ) -> InterpResult<'tcx> {
         // Make sure all the constants required by this frame evaluate successfully (post-monomorphization check).
-        for &const_ in &body.required_consts {
+        for &const_ in body.required_consts() {
             let c =
                 self.instantiate_from_current_frame_and_normalize_erasing_regions(const_.const_)?;
             c.eval(*self.tcx, self.param_env, const_.span).map_err(|err| {

compiler/rustc_middle/src/mir/mod.rs

@@ -383,23 +383,25 @@ pub struct Body<'tcx> {
 
     /// Constants that are required to evaluate successfully for this MIR to be well-formed.
     /// We hold in this field all the constants we are not able to evaluate yet.
+    /// `None` indicates that the list has not been computed yet.
     ///
     /// This is soundness-critical, we make a guarantee that all consts syntactically mentioned in a
     /// function have successfully evaluated if the function ever gets executed at runtime.
-    pub required_consts: Vec<ConstOperand<'tcx>>,
+    pub required_consts: Option<Vec<ConstOperand<'tcx>>>,
 
     /// Further items that were mentioned in this function and hence *may* become monomorphized,
     /// depending on optimizations. We use this to avoid optimization-dependent compile errors: the
     /// collector recursively traverses all "mentioned" items and evaluates all their
     /// `required_consts`.
+    /// `None` indicates that the list has not been computed yet.
     ///
     /// This is *not* soundness-critical and the contents of this list are *not* a stable guarantee.
     /// All that's relevant is that this set is optimization-level-independent, and that it includes
     /// everything that the collector would consider "used". (For example, we currently compute this
     /// set after drop elaboration, so some drop calls that can never be reached are not considered
     /// "mentioned".) See the documentation of `CollectionMode` in
     /// `compiler/rustc_monomorphize/src/collector.rs` for more context.
-    pub mentioned_items: Vec<Spanned<MentionedItem<'tcx>>>,
+    pub mentioned_items: Option<Vec<Spanned<MentionedItem<'tcx>>>>,
 
     /// Does this body use generic parameters. This is used for the `ConstEvaluatable` check.
     ///
@@ -477,8 +479,8 @@ impl<'tcx> Body<'tcx> {
             spread_arg: None,
             var_debug_info,
             span,
-            required_consts: Vec::new(),
-            mentioned_items: Vec::new(),
+            required_consts: None,
+            mentioned_items: None,
             is_polymorphic: false,
             injection_phase: None,
             tainted_by_errors,
@@ -507,8 +509,8 @@ impl<'tcx> Body<'tcx> {
             arg_count: 0,
             spread_arg: None,
             span: DUMMY_SP,
-            required_consts: Vec::new(),
-            mentioned_items: Vec::new(),
+            required_consts: None,
+            mentioned_items: None,
             var_debug_info: Vec::new(),
             is_polymorphic: false,
             injection_phase: None,
@@ -785,6 +787,40 @@ impl<'tcx> Body<'tcx> {
         // No inlined `SourceScope`s, or all of them were `#[track_caller]`.
         caller_location.unwrap_or_else(|| from_span(source_info.span))
     }
+
+    #[track_caller]
+    pub fn set_required_consts(&mut self, required_consts: Vec<ConstOperand<'tcx>>) {
+        assert!(
+            self.required_consts.is_none(),
+            "required_consts for {:?} have already been set",
+            self.source.def_id()
+        );
+        self.required_consts = Some(required_consts);
+    }
+    #[track_caller]
+    pub fn required_consts(&self) -> &[ConstOperand<'tcx>] {
+        match &self.required_consts {
+            Some(l) => l,
+            None => panic!("required_consts for {:?} have not yet been set", self.source.def_id()),
+        }
+    }
+
+    #[track_caller]
+    pub fn set_mentioned_items(&mut self, mentioned_items: Vec<Spanned<MentionedItem<'tcx>>>) {
+        assert!(
+            self.mentioned_items.is_none(),
+            "mentioned_items for {:?} have already been set",
+            self.source.def_id()
+        );
+        self.mentioned_items = Some(mentioned_items);
+    }
+    #[track_caller]
+    pub fn mentioned_items(&self) -> &[Spanned<MentionedItem<'tcx>>] {
+        match &self.mentioned_items {
+            Some(l) => l,
+            None => panic!("mentioned_items for {:?} have not yet been set", self.source.def_id()),
+        }
+    }
 }
 
 impl<'tcx> Index<BasicBlock> for Body<'tcx> {

compiler/rustc_middle/src/mir/visit.rs

@@ -1066,9 +1066,11 @@ macro_rules! super_body {
 
         $self.visit_span($(& $mutability)? $body.span);
 
-        for const_ in &$($mutability)? $body.required_consts {
-            let location = Location::START;
-            $self.visit_const_operand(const_, location);
+        if let Some(required_consts) = &$($mutability)? $body.required_consts {
+            for const_ in required_consts {
+                let location = Location::START;
+                $self.visit_const_operand(const_, location);
+            }
         }
     }
 }

compiler/rustc_mir_build/src/build/custom/mod.rs

@@ -54,8 +54,8 @@ pub(super) fn build_custom_mir<'tcx>(
         spread_arg: None,
         var_debug_info: Vec::new(),
         span,
-        required_consts: Vec::new(),
-        mentioned_items: Vec::new(),
+        required_consts: None,
+        mentioned_items: None,
         is_polymorphic: false,
         tainted_by_errors: None,
         injection_phase: None,

compiler/rustc_mir_transform/src/inline.rs

@@ -744,8 +744,8 @@ impl<'tcx> Inliner<'tcx> {
         // Copy required constants from the callee_body into the caller_body. Although we are only
         // pushing unevaluated consts to `required_consts`, here they may have been evaluated
         // because we are calling `instantiate_and_normalize_erasing_regions` -- so we filter again.
-        caller_body.required_consts.extend(
-            callee_body.required_consts.into_iter().filter(|ct| ct.const_.is_required_const()),
+        caller_body.required_consts.as_mut().unwrap().extend(
+            callee_body.required_consts().into_iter().filter(|ct| ct.const_.is_required_const()),
         );
         // Now that we incorporated the callee's `required_consts`, we can remove the callee from
         // `mentioned_items` -- but we have to take their `mentioned_items` in return. This does
@@ -755,12 +755,11 @@ impl<'tcx> Inliner<'tcx> {
         // We need to reconstruct the `required_item` for the callee so that we can find and
         // remove it.
         let callee_item = MentionedItem::Fn(func.ty(caller_body, self.tcx));
-        if let Some(idx) =
-            caller_body.mentioned_items.iter().position(|item| item.node == callee_item)
-        {
+        let caller_mentioned_items = caller_body.mentioned_items.as_mut().unwrap();
+        if let Some(idx) = caller_mentioned_items.iter().position(|item| item.node == callee_item) {
             // We found the callee, so remove it and add its items instead.
-            caller_body.mentioned_items.remove(idx);
-            caller_body.mentioned_items.extend(callee_body.mentioned_items);
+            caller_mentioned_items.remove(idx);
+            caller_mentioned_items.extend(callee_body.mentioned_items());
         } else {
             // If we can't find the callee, there's no point in adding its items. Probably it
             // already got removed by being inlined elsewhere in the same function, so we already

compiler/rustc_mir_transform/src/lib.rs

@@ -28,11 +28,10 @@ use rustc_hir::def::DefKind;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_index::IndexVec;
-use rustc_middle::mir::visit::Visitor as _;
 use rustc_middle::mir::{
-    traversal, AnalysisPhase, Body, CallSource, ClearCrossCrate, ConstOperand, ConstQualifs,
-    LocalDecl, MirPass, MirPhase, Operand, Place, ProjectionElem, Promoted, RuntimePhase, Rvalue,
-    SourceInfo, Statement, StatementKind, TerminatorKind, START_BLOCK,
+    AnalysisPhase, Body, CallSource, ClearCrossCrate, ConstOperand, ConstQualifs, LocalDecl,
+    MirPass, MirPhase, Operand, Place, ProjectionElem, Promoted, RuntimePhase, Rvalue, SourceInfo,
+    Statement, StatementKind, TerminatorKind, START_BLOCK,
 };
 use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
 use rustc_middle::util::Providers;
@@ -339,12 +338,15 @@ fn mir_promoted(
 
     // Collect `required_consts` *before* promotion, so if there are any consts being promoted
     // we still add them to the list in the outer MIR body.
-    let mut required_consts = Vec::new();
-    let mut required_consts_visitor = RequiredConstsVisitor::new(&mut required_consts);
-    for (bb, bb_data) in traversal::reverse_postorder(&body) {
-        required_consts_visitor.visit_basic_block_data(bb, bb_data);
+    RequiredConstsVisitor::compute_required_consts(&mut body);
+    // If this has an associated by-move async closure body, that doesn't get run through these
+    // passes itself, it gets "tagged along" by the pass manager. `RequiredConstsVisitor` is not
+    // a regular pass so we have to also apply it manually to the other body.
+    if let Some(coroutine) = body.coroutine.as_mut() {
+        if let Some(by_move_body) = coroutine.by_move_body.as_mut() {
+            RequiredConstsVisitor::compute_required_consts(by_move_body);
+        }
     }
-    body.required_consts = required_consts;
 
     // What we need to run borrowck etc.
     let promote_pass = promote_consts::PromoteTemps::default();
@@ -561,9 +563,6 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
         tcx,
         body,
         &[
-            // Before doing anything, remember which items are being mentioned so that the set of items
-            // visited does not depend on the optimization level.
-            &mentioned_items::MentionedItems,
             // Add some UB checks before any UB gets optimized away.
             &check_alignment::CheckAlignment,
             // Before inlining: trim down MIR with passes to reduce inlining work.
@@ -657,6 +656,19 @@ fn inner_optimized_mir(tcx: TyCtxt<'_>, did: LocalDefId) -> Body<'_> {
         return body;
     }
 
+    // Before doing anything, remember which items are being mentioned so that the set of items
+    // visited does not depend on the optimization level.
+    // We do not use `run_passes` for this as that might skip the pass if `injection_phase` is set.
+    mentioned_items::MentionedItems.run_pass(tcx, &mut body);
+    // If this has an associated by-move async closure body, that doesn't get run through these
+    // passes itself, it gets "tagged along" by the pass manager. Since we're not using the pass
+    // manager we have to do this by hand.
+    if let Some(coroutine) = body.coroutine.as_mut() {
+        if let Some(by_move_body) = coroutine.by_move_body.as_mut() {
+            mentioned_items::MentionedItems.run_pass(tcx, by_move_body);
+        }
+    }
+
     // If `mir_drops_elaborated_and_const_checked` found that the current body has unsatisfiable
     // predicates, it will shrink the MIR to a single `unreachable` terminator.
     // More generally, if MIR is a lone `unreachable`, there is nothing to optimize.

compiler/rustc_mir_transform/src/mentioned_items.rs

@@ -23,10 +23,9 @@ impl<'tcx> MirPass<'tcx> for MentionedItems {
     }
 
     fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut mir::Body<'tcx>) {
-        debug_assert!(body.mentioned_items.is_empty());
         let mut mentioned_items = Vec::new();
         MentionedItemsVisitor { tcx, body, mentioned_items: &mut mentioned_items }.visit_body(body);
-        body.mentioned_items = mentioned_items;
+        body.set_mentioned_items(mentioned_items);
     }
 }
 

compiler/rustc_mir_transform/src/promote_consts.rs

@@ -702,6 +702,9 @@ struct Promoter<'a, 'tcx> {
     temps: &'a mut IndexVec<Local, TempState>,
     extra_statements: &'a mut Vec<(Location, Statement<'tcx>)>,
 
+    /// Used to assemble the required_consts list while building the promoted.
+    required_consts: Vec<ConstOperand<'tcx>>,
+
     /// If true, all nested temps are also kept in the
     /// source MIR, not moved to the promoted MIR.
     keep_original: bool,
@@ -924,11 +927,14 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
         let span = self.promoted.span;
         self.assign(RETURN_PLACE, rvalue, span);
 
-        // Now that we did promotion, we know whether we'll want to add this to `required_consts`.
+        // Now that we did promotion, we know whether we'll want to add this to `required_consts` of
+        // the surrounding MIR body.
         if self.add_to_required {
-            self.source.required_consts.push(promoted_op);
+            self.source.required_consts.as_mut().unwrap().push(promoted_op);
         }
 
+        self.promoted.set_required_consts(self.required_consts);
+
         self.promoted
     }
 }
@@ -947,7 +953,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Promoter<'a, 'tcx> {
 
     fn visit_const_operand(&mut self, constant: &mut ConstOperand<'tcx>, _location: Location) {
         if constant.const_.is_required_const() {
-            self.promoted.required_consts.push(*constant);
+            self.required_consts.push(*constant);
         }
 
         // Skipping `super_constant` as the visitor is otherwise only looking for locals.
@@ -1011,9 +1017,9 @@ fn promote_candidates<'tcx>(
             extra_statements: &mut extra_statements,
             keep_original: false,
             add_to_required: false,
+            required_consts: Vec::new(),
         };
 
-        // `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);

compiler/rustc_mir_transform/src/required_consts.rs

@@ -1,14 +1,23 @@
 use rustc_middle::mir::visit::Visitor;
-use rustc_middle::mir::{ConstOperand, Location};
+use rustc_middle::mir::{traversal, Body, ConstOperand, Location};
 
 pub struct RequiredConstsVisitor<'a, 'tcx> {
     required_consts: &'a mut Vec<ConstOperand<'tcx>>,
 }
 
 impl<'a, 'tcx> RequiredConstsVisitor<'a, 'tcx> {
-    pub fn new(required_consts: &'a mut Vec<ConstOperand<'tcx>>) -> Self {
+    fn new(required_consts: &'a mut Vec<ConstOperand<'tcx>>) -> Self {
         RequiredConstsVisitor { required_consts }
     }
+
+    pub fn compute_required_consts(body: &mut Body<'tcx>) {
+        let mut required_consts = Vec::new();
+        let mut required_consts_visitor = RequiredConstsVisitor::new(&mut required_consts);
+        for (bb, bb_data) in traversal::reverse_postorder(&body) {
+            required_consts_visitor.visit_basic_block_data(bb, bb_data);
+        }
+        body.set_required_consts(required_consts);
+    }
 }
 
 impl<'tcx> Visitor<'tcx> for RequiredConstsVisitor<'_, 'tcx> {

compiler/rustc_mir_transform/src/shim.rs

@@ -305,7 +305,7 @@ fn new_body<'tcx>(
     arg_count: usize,
     span: Span,
 ) -> Body<'tcx> {
-    Body::new(
+    let mut body = Body::new(
         source,
         basic_blocks,
         IndexVec::from_elem_n(
@@ -326,7 +326,10 @@ fn new_body<'tcx>(
         None,
         // FIXME(compiler-errors): is this correct?
         None,
-    )
+    );
+    // Shims do not directly mention any consts.
+    body.set_required_consts(Vec::new());
+    body
 }
 
 pub struct DropShimElaborator<'a, 'tcx> {
@@ -969,13 +972,16 @@ pub fn build_adt_ctor(tcx: TyCtxt<'_>, ctor_id: DefId) -> Body<'_> {
     };
 
     let source = MirSource::item(ctor_id);
-    let body = new_body(
+    let mut body = new_body(
         source,
         IndexVec::from_elem_n(start_block, 1),
         local_decls,
         sig.inputs().len(),
         span,
     );
+    // A constructor doesn't mention any other items (and we don't run the usual optimization passes
+    // so this would otherwise not get filled).
+    body.set_mentioned_items(Vec::new());
 
     crate::pass_manager::dump_mir_for_phase_change(tcx, &body);