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

Author: RalfJung

Diff for: compiler/rustc_monomorphize/src/collector.rs

@@ -429,7 +429,9 @@ fn collect_items_rec<'tcx>(
         }
         MonoItem::Fn(instance) => {
             // Sanity check whether this ended up being collected accidentally
-            debug_assert!(should_codegen_locally(tcx, &instance));
+            if mode == CollectionMode::UsedItems {
+                debug_assert!(should_codegen_locally(tcx, &instance));
+            }
 
             // Keep track of the monomorphization recursion depth
             recursion_depth_reset = Some(check_recursion_limit(
@@ -1518,16 +1520,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(());
 
@@ -1542,21 +1554,32 @@ 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, CollectionMode::UsedItems))
+        .chain(
+            mentioned_roots
+                .into_iter()
+                .map(|mono_item| (mono_item, CollectionMode::MentionedItems)),
+        )
+        .filter_map(|(Spanned { node: mono_item, .. }, mode)| {
+            mono_item.is_instantiable(tcx).then_some((mono_item, mode))
         })
         .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)>,
 }
 
@@ -1570,33 +1593,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 => {
@@ -1612,31 +1635,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;
+        }
+        // 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 {
+            return Some(CollectionMode::UsedItems);
+        }
+        // We have to skip `must_be_overridden` bodies; asking for their "mentioned" items
+        // leads to ICEs.
+        if self.tcx.intrinsic(def_id).is_some_and(|i| i.must_be_overridden) {
+            return None;
+        }
+        // Everything else is considered "mentioned"
+        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),
+            }
         }
     }
 
@@ -1672,7 +1718,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));
     }
 }
 
@@ -1777,15 +1823,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,
                 );
             });
         });

Diff for: tests/ui/consts/required-consts/collect-roots-dead-fn.noopt.stderr

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

Diff for: tests/ui/consts/required-consts/collect-roots-dead-fn.opt.stderr

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

Diff for: tests/ui/consts/required-consts/collect-roots-dead-fn.rs

@@ -0,0 +1,26 @@
+//@revisions: noopt opt
+//@ build-fail
+//@[noopt] compile-flags: -Copt-level=0
+//@[opt] compile-flags: -O
+
+//! This used to fail in optimized builds but somehow unoptimized builds
+//! never collected `g` so the const-eval error never happened.
+#![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>()
+}
+
+fn h<T>() {
+    Fail::<T>::C;
+}

Diff for: 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.
+//! (Curcially, behavior is still the same in both cases.)
+#![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;
+}

Diff for: tests/ui/consts/required-consts/collect-roots-inline-fn.noopt.stderr

@@ -0,0 +1,23 @@
+error[E0080]: evaluation of `Zst::<u32>::ASSERT` failed
+  --> $DIR/collect-roots-inline-fn.rs:13:9
+   |
+LL |         panic!();
+   |         ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-roots-inline-fn.rs:13:9
+   |
+   = 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-inline-fn.rs:18:5
+   |
+LL |     Zst::<T>::ASSERT;
+   |     ^^^^^^^^^^^^^^^^
+
+note: the above error was encountered while instantiating `fn f::<u32>`
+  --> $DIR/collect-roots-inline-fn.rs:22:5
+   |
+LL |     f::<u32>()
+   |     ^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

Diff for: tests/ui/consts/required-consts/collect-roots-inline-fn.opt.stderr

@@ -0,0 +1,17 @@
+error[E0080]: evaluation of `Zst::<u32>::ASSERT` failed
+  --> $DIR/collect-roots-inline-fn.rs:13:9
+   |
+LL |         panic!();
+   |         ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-roots-inline-fn.rs:13:9
+   |
+   = 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-inline-fn.rs:18:5
+   |
+LL |     Zst::<T>::ASSERT;
+   |     ^^^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

Diff for: tests/ui/consts/required-consts/collect-roots-inline-fn.rs

@@ -0,0 +1,23 @@
+//@revisions: noopt opt
+//@ build-fail
+//@[noopt] compile-flags: -Copt-level=0
+//@[opt] compile-flags: -O
+
+//! In optimized builds, the functions in this crate are all marked "inline" so none of them become
+//! collector roots. Ensure that we still evaluate the constants.
+#![crate_type = "lib"]
+
+struct Zst<T>(T);
+impl<T> Zst<T> {
+    const ASSERT: () = if std::mem::size_of::<T>() != 0 {
+        panic!(); //~ERROR: evaluation of `Zst::<u32>::ASSERT` failed
+    };
+}
+
+fn f<T>() {
+    Zst::<T>::ASSERT;
+}
+
+pub fn g() {
+    f::<u32>()
+}