Auto merge of rust-lang#122862 - RalfJung:collect-all-mono, r=<try>

collector: always consider all monomorphic functions to be 'mentioned'

This would fix rust-lang#122814. But it's probably not going to be cheap...

Ideally we'd avoid building the optimized MIR for these new roots, and only request `mir_drops_elaborated_and_const_checked` -- but that MIR is often getting stolen so I don't see a way to do that.

TODO before landing:
- [ ] Figure out if there is a testcase [here](rust-lang#122814 (comment)).

r? `@oli-obk` `@tmiasko`

Author: bors

compiler/rustc_monomorphize/src/collector.rs

@@ -1513,16 +1513,26 @@ fn collect_const_value<'tcx>(
 // Find all non-generic items by walking the HIR. These items serve as roots to
 // start monomorphizing from.
 #[instrument(skip(tcx, mode), level = "debug")]
-fn collect_roots(tcx: TyCtxt<'_>, mode: MonoItemCollectionStrategy) -> Vec<MonoItem<'_>> {
+fn collect_roots(
+    tcx: TyCtxt<'_>,
+    mode: MonoItemCollectionStrategy,
+) -> Vec<(MonoItem<'_>, CollectionMode)> {
     debug!("collecting roots");
-    let mut roots = Vec::new();
+    let mut used_roots = MonoItems::new();
+    let mut mentioned_roots = MonoItems::new();
 
     {
         let entry_fn = tcx.entry_fn(());
 
         debug!("collect_roots: entry_fn = {:?}", entry_fn);
 
-        let mut collector = RootCollector { tcx, strategy: mode, entry_fn, output: &mut roots };
+        let mut collector = RootCollector {
+            tcx,
+            strategy: mode,
+            entry_fn,
+            used_roots: &mut used_roots,
+            mentioned_roots: &mut mentioned_roots,
+        };
 
         let crate_items = tcx.hir_crate_items(());
 
@@ -1537,21 +1547,30 @@ fn collect_roots(tcx: TyCtxt<'_>, mode: MonoItemCollectionStrategy) -> Vec<MonoI
         collector.push_extra_entry_roots();
     }
 
+    // Chain the two root lists together. Used items go first, to make it
+    // more likely that when we visit a mentioned item, we can stop immediately as it was already used.
     // We can only codegen items that are instantiable - items all of
     // whose predicates hold. Luckily, items that aren't instantiable
     // can't actually be used, so we can just skip codegenning them.
-    roots
+    used_roots
         .into_iter()
-        .filter_map(|Spanned { node: mono_item, .. }| {
-            mono_item.is_instantiable(tcx).then_some(mono_item)
-        })
+        .map(|mono_item| (mono_item.node, CollectionMode::UsedItems))
+        .chain(
+            mentioned_roots
+                .into_iter()
+                .map(|mono_item| (mono_item.node, CollectionMode::MentionedItems)),
+        )
+        .filter(|(mono_item, _mode)| mono_item.is_instantiable(tcx))
         .collect()
 }
 
 struct RootCollector<'a, 'tcx> {
     tcx: TyCtxt<'tcx>,
     strategy: MonoItemCollectionStrategy,
-    output: &'a mut MonoItems<'tcx>,
+    // `MonoItems` includes spans we don't actually want... but this lets us reuse some of the
+    // collector's functions.
+    used_roots: &'a mut MonoItems<'tcx>,
+    mentioned_roots: &'a mut MonoItems<'tcx>,
     entry_fn: Option<(DefId, EntryFnType)>,
 }
 
@@ -1565,33 +1584,33 @@ impl<'v> RootCollector<'_, 'v> {
                     debug!("RootCollector: ADT drop-glue for `{id:?}`",);
 
                     let ty = self.tcx.type_of(id.owner_id.to_def_id()).no_bound_vars().unwrap();
-                    visit_drop_use(self.tcx, ty, true, DUMMY_SP, self.output);
+                    visit_drop_use(self.tcx, ty, true, DUMMY_SP, self.used_roots);
                 }
             }
             DefKind::GlobalAsm => {
                 debug!(
                     "RootCollector: ItemKind::GlobalAsm({})",
                     self.tcx.def_path_str(id.owner_id)
                 );
-                self.output.push(dummy_spanned(MonoItem::GlobalAsm(id)));
+                self.used_roots.push(dummy_spanned(MonoItem::GlobalAsm(id)));
             }
             DefKind::Static { .. } => {
                 let def_id = id.owner_id.to_def_id();
                 debug!("RootCollector: ItemKind::Static({})", self.tcx.def_path_str(def_id));
-                self.output.push(dummy_spanned(MonoItem::Static(def_id)));
+                self.used_roots.push(dummy_spanned(MonoItem::Static(def_id)));
             }
             DefKind::Const => {
                 // const items only generate mono items if they are
                 // actually used somewhere. Just declaring them is insufficient.
 
                 // but even just declaring them must collect the items they refer to
                 if let Ok(val) = self.tcx.const_eval_poly(id.owner_id.to_def_id()) {
-                    collect_const_value(self.tcx, val, self.output);
+                    collect_const_value(self.tcx, val, self.used_roots);
                 }
             }
             DefKind::Impl { .. } => {
                 if self.strategy == MonoItemCollectionStrategy::Eager {
-                    create_mono_items_for_default_impls(self.tcx, id, self.output);
+                    create_mono_items_for_default_impls(self.tcx, id, self.used_roots);
                 }
             }
             DefKind::Fn => {
@@ -1607,31 +1626,54 @@ impl<'v> RootCollector<'_, 'v> {
         }
     }
 
-    fn is_root(&self, def_id: LocalDefId) -> bool {
-        !self.tcx.generics_of(def_id).requires_monomorphization(self.tcx)
-            && match self.strategy {
-                MonoItemCollectionStrategy::Eager => true,
-                MonoItemCollectionStrategy::Lazy => {
-                    self.entry_fn.and_then(|(id, _)| id.as_local()) == Some(def_id)
-                        || self.tcx.is_reachable_non_generic(def_id)
-                        || self
-                            .tcx
-                            .codegen_fn_attrs(def_id)
-                            .flags
-                            .contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL)
-                }
+    /// Determines whether this is an item we start walking, and in which mode. The "real" roots are
+    /// walked as "used" items, but that set is optimization-dependent. We add all other non-generic
+    /// items as "mentioned" roots. This makes the set of items where `is_root` return `Some`
+    /// optimization-independent, which is crucial to ensure that optimized and unoptimized builds
+    /// evaluate the same constants.
+    fn is_root(&self, def_id: LocalDefId) -> Option<CollectionMode> {
+        // Generic things are never roots.
+        if self.tcx.generics_of(def_id).requires_monomorphization(self.tcx) {
+            return None;
+        }
+        // We have to skip `must_be_overridden` bodies; asking for their MIR ICEs.
+        if self.tcx.intrinsic(def_id).is_some_and(|i| i.must_be_overridden) {
+            return None;
+        }
+        // The rest is definitely a root, but is it used or merely mentioned?
+        // Determine whether this item is reachable, which makes it "used".
+        let is_used_root = match self.strategy {
+            MonoItemCollectionStrategy::Eager => true,
+            MonoItemCollectionStrategy::Lazy => {
+                self.entry_fn.and_then(|(id, _)| id.as_local()) == Some(def_id)
+                    || self.tcx.is_reachable_non_generic(def_id)
+                    || self
+                        .tcx
+                        .codegen_fn_attrs(def_id)
+                        .flags
+                        .contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL)
             }
+        };
+        if is_used_root {
+            Some(CollectionMode::UsedItems)
+        } else {
+            Some(CollectionMode::MentionedItems)
+        }
     }
 
     /// If `def_id` represents a root, pushes it onto the list of
     /// outputs. (Note that all roots must be monomorphic.)
     #[instrument(skip(self), level = "debug")]
     fn push_if_root(&mut self, def_id: LocalDefId) {
-        if self.is_root(def_id) {
+        if let Some(mode) = self.is_root(def_id) {
             debug!("found root");
 
             let instance = Instance::mono(self.tcx, def_id.to_def_id());
-            self.output.push(create_fn_mono_item(self.tcx, instance, DUMMY_SP));
+            let mono_item = create_fn_mono_item(self.tcx, instance, DUMMY_SP);
+            match mode {
+                CollectionMode::UsedItems => self.used_roots.push(mono_item),
+                CollectionMode::MentionedItems => self.mentioned_roots.push(mono_item),
+            }
         }
     }
 
@@ -1667,7 +1709,7 @@ impl<'v> RootCollector<'_, 'v> {
             self.tcx.mk_args(&[main_ret_ty.into()]),
         );
 
-        self.output.push(create_fn_mono_item(self.tcx, start_instance, DUMMY_SP));
+        self.used_roots.push(create_fn_mono_item(self.tcx, start_instance, DUMMY_SP));
     }
 }
 
@@ -1772,15 +1814,15 @@ pub fn collect_crate_mono_items(
         let state: LRef<'_, _> = &mut state;
 
         tcx.sess.time("monomorphization_collector_graph_walk", || {
-            par_for_each_in(roots, |root| {
+            par_for_each_in(roots, |(root, mode)| {
                 let mut recursion_depths = DefIdMap::default();
                 collect_items_rec(
                     tcx,
                     dummy_spanned(root),
                     state,
                     &mut recursion_depths,
                     recursion_limit,
-                    CollectionMode::UsedItems,
+                    mode,
                 );
             });
         });

tests/incremental/callee_caller_cross_crate/b.rs

@@ -6,7 +6,7 @@
 
 extern crate a;
 
-#[rustc_clean(except="typeck", cfg="rpass2")]
+#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
 pub fn call_function0() {
     a::function0(77);
 }

tests/incremental/dirty_clean.rs

@@ -36,6 +36,7 @@ mod y {
         //[cfail2]~| ERROR `type_of(y)` should be dirty but is not
         //[cfail2]~| ERROR `fn_sig(y)` should be dirty but is not
         //[cfail2]~| ERROR `typeck(y)` should be clean but is not
+        //[cfail2]~| ERROR `optimized_mir(y)` should be clean but is not
         x::x();
     }
 }

tests/incremental/hashes/call_expressions.rs

@@ -62,9 +62,9 @@ mod change_callee_indirectly_function {
     #[cfg(not(any(cfail1,cfail4)))]
     use super::callee2 as callee;
 
-    #[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail2")]
+    #[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
     #[rustc_clean(cfg="cfail3")]
-    #[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail5")]
+    #[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail5")]
     #[rustc_clean(cfg="cfail6")]
     pub fn change_callee_indirectly_function() {
         callee(1, 2)

tests/incremental/hashes/indexing_expressions.rs

@@ -23,9 +23,9 @@ fn change_simple_index(slice: &[u32]) -> u32 {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail2")]
+#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail2")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail5")]
+#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail5")]
 #[rustc_clean(cfg="cfail6")]
 fn change_simple_index(slice: &[u32]) -> u32 {
     slice[4]
@@ -40,9 +40,9 @@ fn change_lower_bound(slice: &[u32]) -> &[u32] {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail2")]
+#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail2")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail5")]
+#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail5")]
 #[rustc_clean(cfg="cfail6")]
 fn change_lower_bound(slice: &[u32]) -> &[u32] {
     &slice[2..5]
@@ -57,9 +57,9 @@ fn change_upper_bound(slice: &[u32]) -> &[u32] {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail2")]
+#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail2")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(except="opt_hir_owner_nodes", cfg="cfail5")]
+#[rustc_clean(except="opt_hir_owner_nodes,optimized_mir", cfg="cfail5")]
 #[rustc_clean(cfg="cfail6")]
 fn change_upper_bound(slice: &[u32]) -> &[u32] {
     &slice[3..7]
@@ -74,9 +74,9 @@ fn add_lower_bound(slice: &[u32]) -> &[u32] {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail2")]
+#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail5")]
+#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail5")]
 #[rustc_clean(cfg="cfail6")]
 fn add_lower_bound(slice: &[u32]) -> &[u32] {
     &slice[3..4]
@@ -91,9 +91,9 @@ fn add_upper_bound(slice: &[u32]) -> &[u32] {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail2")]
+#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail5")]
+#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail5")]
 #[rustc_clean(cfg="cfail6")]
 fn add_upper_bound(slice: &[u32]) -> &[u32] {
     &slice[3..7]
@@ -108,9 +108,9 @@ fn change_mutability(slice: &mut [u32]) -> u32 {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail2")]
+#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail5")]
+#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail5")]
 #[rustc_clean(cfg="cfail6")]
 fn change_mutability(slice: &mut [u32]) -> u32 {
     (&    slice[3..5])[0]
@@ -125,9 +125,9 @@ fn exclusive_to_inclusive_range(slice: &[u32]) -> &[u32] {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail2")]
+#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="cfail5")]
+#[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="cfail5")]
 #[rustc_clean(cfg="cfail6")]
 fn exclusive_to_inclusive_range(slice: &[u32]) -> &[u32] {
     &slice[3..=7]

tests/incremental/ich_method_call_trait_scope.rs

@@ -26,7 +26,7 @@ mod mod3 {
     #[cfg(rpass2)]
     use Trait2;
 
-    #[rustc_clean(except="typeck", cfg="rpass2")]
+    #[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
     fn bar() {
         ().method();
     }

tests/incremental/ich_resolve_results.rs

@@ -36,7 +36,7 @@ mod mod3 {
     }
 
     #[rustc_clean(cfg="rpass2")]
-    #[rustc_clean(except="opt_hir_owner_nodes,typeck", cfg="rpass3")]
+    #[rustc_clean(except="opt_hir_owner_nodes,typeck,optimized_mir", cfg="rpass3")]
     fn in_type() {
         test::<Foo>();
     }

tests/incremental/struct_add_field.rs

@@ -21,12 +21,12 @@ pub struct Y {
     pub y: char
 }
 
-#[rustc_clean(except="fn_sig,typeck", cfg="rpass2")]
+#[rustc_clean(except="fn_sig,typeck,optimized_mir", cfg="rpass2")]
 pub fn use_X(x: X) -> u32 {
     x.x as u32
 }
 
-#[rustc_clean(except="typeck", cfg="rpass2")]
+#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
 pub fn use_EmbedX(embed: EmbedX) -> u32 {
     embed.x.x as u32
 }

tests/incremental/struct_change_field_type.rs

@@ -24,13 +24,13 @@ pub struct Y {
     pub y: char
 }
 
-#[rustc_clean(except="typeck", cfg="rpass2")]
+#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
 pub fn use_X() -> u32 {
     let x: X = X { x: 22 };
     x.x as u32
 }
 
-#[rustc_clean(except="typeck", cfg="rpass2")]
+#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
 pub fn use_EmbedX(x: EmbedX) -> u32 {
     let x: X = X { x: 22 };
     x.x as u32

tests/incremental/struct_change_field_type_cross_crate/b.rs

@@ -8,13 +8,13 @@ extern crate a;
 
 use a::*;
 
-#[rustc_clean(except="typeck", cfg="rpass2")]
+#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
 pub fn use_X() -> u32 {
     let x: X = X { x: 22 };
     x.x as u32
 }
 
-#[rustc_clean(except="typeck", cfg="rpass2")]
+#[rustc_clean(except="typeck,optimized_mir", cfg="rpass2")]
 pub fn use_EmbedX(embed: EmbedX) -> u32 {
     embed.x.x as u32
 }