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

Author: RalfJung

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/struct_remove_field.rs

@@ -1,4 +1,4 @@
-// Test incremental compilation tracking where we change field names
+// Test incremental compilation tracking where we remove a field
 // in between revisions (hashing should be stable).
 
 //@ revisions:rpass1 rpass2

tests/ui/consts/required-consts/collect-roots-dead-fn1.noopt.stderr

@@ -0,0 +1,23 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/collect-roots-dead-fn1.rs:15:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-roots-dead-fn1.rs:15: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-roots-dead-fn1.rs:31:5
+   |
+LL |     Fail::<T>::C;
+   |     ^^^^^^^^^^^^
+
+note: the above error was encountered while instantiating `fn h::<i32>`
+  --> $DIR/collect-roots-dead-fn1.rs:24:5
+   |
+LL |     h::<i32>()
+   |     ^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/required-consts/collect-roots-dead-fn1.opt.stderr

@@ -0,0 +1,23 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/collect-roots-dead-fn1.rs:15:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-roots-dead-fn1.rs:15: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-roots-dead-fn1.rs:31:5
+   |
+LL |     Fail::<T>::C;
+   |     ^^^^^^^^^^^^
+
+note: the above error was encountered while instantiating `fn h::<i32>`
+  --> $DIR/collect-roots-dead-fn1.rs:24:5
+   |
+LL |     h::<i32>()
+   |     ^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/required-consts/collect-roots-dead-fn1.rs

@@ -0,0 +1,32 @@
+//@revisions: noopt opt
+//@ build-fail
+//@[noopt] compile-flags: -Copt-level=0
+//@[opt] compile-flags: -O
+
+//! This used to fail in optimized builds but pass in unoptimized builds. The reason is that in
+//! optimized builds, `f` gets marked as cross-crate-inlineable, so the functions it calls become
+//! reachable, and therefore `g` becomes a collection root. But in unoptimized builds, `g` is no
+//! root, and the call to `g` disappears in an early `SimplifyCfg` before "mentioned items" are
+//! gathered, so we never reach `g`.
+#![crate_type = "lib"]
+
+struct Fail<T>(T);
+impl<T> Fail<T> {
+    const C: () = panic!(); //~ERROR: evaluation of `Fail::<i32>::C` failed
+}
+
+pub fn f() {
+    loop {}; g()
+}
+
+#[inline(never)]
+fn g() {
+    h::<i32>()
+}
+
+// Make sure we only use the faulty const in a generic function, or
+// else it gets evaluated by some MIR pass.
+#[inline(never)]
+fn h<T>() {
+    Fail::<T>::C;
+}

tests/ui/consts/required-consts/collect-roots-dead-fn2.rs

@@ -0,0 +1,23 @@
+//@revisions: noopt opt
+//@ build-pass
+//@[noopt] compile-flags: -Copt-level=0
+//@[opt] compile-flags: -O
+
+//! A slight variant of `collect-roots-in-dead-fn` where the dead call is itself generic. Now this
+//! *passes* in both optimized and unoptimized builds: the call to `h` always disappears in an early
+//! `SimplifyCfg`, and `h` is generic so it can never be a root.
+#![crate_type = "lib"]
+
+struct Fail<T>(T);
+impl<T> Fail<T> {
+    const C: () = panic!();
+}
+
+pub fn f() {
+    loop {}; h::<i32>()
+}
+
+#[inline(never)]
+fn h<T>() {
+    Fail::<T>::C;
+}

tests/ui/consts/required-consts/collect-roots-dead-fn3.noopt.stderr

@@ -0,0 +1,23 @@
+error[E0080]: evaluation of `fail::<()>::{constant#0}` failed
+  --> $DIR/collect-roots-dead-fn3.rs:35:13
+   |
+LL |     const { panic!(); }
+   |             ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-roots-dead-fn3.rs:35:13
+   |
+   = 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-roots-dead-fn3.rs:35:5
+   |
+LL |     const { panic!(); }
+   |     ^^^^^^^^^^^^^^^^^^^
+
+note: the above error was encountered while instantiating `fn fail::<()>`
+  --> $DIR/collect-roots-dead-fn3.rs:28:5
+   |
+LL |     fail::<()>();
+   |     ^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/required-consts/collect-roots-dead-fn3.opt.stderr

@@ -0,0 +1,23 @@
+error[E0080]: evaluation of `fail::<()>::{constant#0}` failed
+  --> $DIR/collect-roots-dead-fn3.rs:35:13
+   |
+LL |     const { panic!(); }
+   |             ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-roots-dead-fn3.rs:35:13
+   |
+   = 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-roots-dead-fn3.rs:35:5
+   |
+LL |     const { panic!(); }
+   |     ^^^^^^^^^^^^^^^^^^^
+
+note: the above error was encountered while instantiating `fn fail::<()>`
+  --> $DIR/collect-roots-dead-fn3.rs:28:5
+   |
+LL |     fail::<()>();
+   |     ^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/required-consts/collect-roots-dead-fn3.rs

@@ -0,0 +1,36 @@
+//@revisions: noopt opt
+//@ build-fail
+//@[noopt] compile-flags: -Copt-level=0
+//@[opt] compile-flags: -O
+
+#![crate_type = "lib"]
+#![feature(inline_const)]
+
+// Will be `inline` with optimizations, so then `g::<()>` becomes reachable. At the same time `g` is
+// not "mentioned" in `f` since it is only called inside an inline `const` and hence never appears
+// in its MIR! This fundamentally is an issue caused by reachability walking the HIR (before inline
+// `const` are extracted) and collection being based on MIR.
+pub fn f() {
+    loop {}; const { g::<()>() }
+}
+
+// When this comes reachable (for any `T`), so does `hidden_root`.
+// And `hidden_root` is monomorphic so it can become a root!
+const fn g<T>() {
+    match std::mem::size_of::<T>() {
+        0 => (),
+        _ => hidden_root(),
+    }
+}
+
+#[inline(never)]
+const fn hidden_root() {
+    fail::<()>();
+}
+
+#[inline(never)]
+const fn fail<T>() {
+    // Hiding this in a generic fn so that it doesn't get evaluated by
+    // MIR passes.
+    const { panic!(); } //~ERROR: evaluation of `fail::<()>::{constant#0}` failed
+}