Auto merge of #122505 - oli-obk:visit_nested_body2, r=<try>

Don't walk the bodies of free constants for reachability.

follow-up to #122371

we don't have generic constants yet, so we don't need to handle failure to eval a free constant.

Associated consts and generic consts will be a different topic, that I will look into in a follow-up

r? `@tmiasko`

Cargo.lock

@@ -4400,6 +4400,7 @@ dependencies = [
  "rustc_lexer",
  "rustc_macros",
  "rustc_middle",
+ "rustc_privacy",
  "rustc_session",
  "rustc_span",
  "rustc_target",

compiler/rustc_middle/src/mir/interpret/queries.rs

@@ -1,4 +1,6 @@
-use super::{ErrorHandled, EvalToConstValueResult, EvalToValTreeResult, GlobalId};
+use super::{
+    ErrorHandled, EvalToAllocationRawResult, EvalToConstValueResult, EvalToValTreeResult, GlobalId,
+};
 
 use crate::mir;
 use crate::query::TyCtxtEnsure;
@@ -12,7 +14,7 @@ use rustc_span::{Span, DUMMY_SP};
 
 impl<'tcx> TyCtxt<'tcx> {
     /// Evaluates a constant without providing any generic parameters. This is useful to evaluate consts
-    /// that can't take any generic arguments like statics, const items or enum discriminants. If a
+    /// that can't take any generic arguments like const items or enum discriminants. If a
     /// generic parameter is used within the constant `ErrorHandled::ToGeneric` will be returned.
     #[instrument(skip(self), level = "debug")]
     pub fn const_eval_poly(self, def_id: DefId) -> EvalToConstValueResult<'tcx> {
@@ -26,6 +28,25 @@ impl<'tcx> TyCtxt<'tcx> {
         let param_env = self.param_env(def_id).with_reveal_all_normalized(self);
         self.const_eval_global_id(param_env, cid, None)
     }
+
+    /// Evaluates a constant without providing any generic parameters. This is useful to evaluate consts
+    /// that can't take any generic arguments like const items or enum discriminants. If a
+    /// generic parameter is used within the constant `ErrorHandled::ToGeneric` will be returned.
+    #[instrument(skip(self), level = "debug")]
+    pub fn const_eval_poly_to_alloc(self, def_id: DefId) -> EvalToAllocationRawResult<'tcx> {
+        // In some situations def_id will have generic parameters within scope, but they aren't allowed
+        // to be used. So we can't use `Instance::mono`, instead we feed unresolved generic parameters
+        // into `const_eval` which will return `ErrorHandled::ToGeneric` if any of them are
+        // encountered.
+        let args = GenericArgs::identity_for_item(self, def_id);
+        let instance = ty::Instance::new(def_id, args);
+        let cid = GlobalId { instance, promoted: None };
+        let param_env = self.param_env(def_id).with_reveal_all_normalized(self);
+        let inputs = self.erase_regions(param_env.and(cid));
+        // The query doesn't know where it is being invoked, so we need to fix the span.
+        self.eval_to_allocation_raw(inputs)
+    }
+
     /// Resolves and evaluates a constant.
     ///
     /// The constant can be located on a trait like `<A as B>::C`, in which case the given
@@ -177,7 +198,7 @@ impl<'tcx> TyCtxt<'tcx> {
 
 impl<'tcx> TyCtxtEnsure<'tcx> {
     /// Evaluates a constant without providing any generic parameters. This is useful to evaluate consts
-    /// that can't take any generic arguments like statics, const items or enum discriminants. If a
+    /// that can't take any generic arguments like const items or enum discriminants. If a
     /// generic parameter is used within the constant `ErrorHandled::ToGeneric` will be returned.
     #[instrument(skip(self), level = "debug")]
     pub fn const_eval_poly(self, def_id: DefId) {

compiler/rustc_passes/Cargo.toml

@@ -18,6 +18,7 @@ rustc_index = { path = "../rustc_index" }
 rustc_lexer = { path = "../rustc_lexer" }
 rustc_macros = { path = "../rustc_macros" }
 rustc_middle = { path = "../rustc_middle" }
+rustc_privacy = { path = "../rustc_privacy" }
 rustc_session = { path = "../rustc_session" }
 rustc_span = { path = "../rustc_span" }
 rustc_target = { path = "../rustc_target" }

compiler/rustc_passes/src/reachable.rs

@@ -6,16 +6,18 @@
 // reachable as well.
 
 use hir::def_id::LocalDefIdSet;
+use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::Node;
 use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
 use rustc_middle::middle::privacy::{self, Level};
-use rustc_middle::mir::interpret::{ConstAllocation, GlobalAlloc};
+use rustc_middle::mir::interpret::{ConstAllocation, ErrorHandled, GlobalAlloc};
 use rustc_middle::query::Providers;
-use rustc_middle::ty::{self, TyCtxt};
+use rustc_middle::ty::{self, ExistentialTraitRef, TyCtxt};
+use rustc_privacy::DefIdVisitor;
 use rustc_session::config::CrateType;
 use rustc_target::spec::abi::Abi;
 
@@ -65,23 +67,8 @@ impl<'tcx> Visitor<'tcx> for ReachableContext<'tcx> {
             _ => None,
         };
 
-        if let Some(res) = res
-            && let Some(def_id) = res.opt_def_id().and_then(|el| el.as_local())
-        {
-            if self.def_id_represents_local_inlined_item(def_id.to_def_id()) {
-                self.worklist.push(def_id);
-            } else {
-                match res {
-                    // Reachable constants and reachable statics can have their contents inlined
-                    // into other crates. Mark them as reachable and recurse into their body.
-                    Res::Def(DefKind::Const | DefKind::AssocConst | DefKind::Static { .. }, _) => {
-                        self.worklist.push(def_id);
-                    }
-                    _ => {
-                        self.reachable_symbols.insert(def_id);
-                    }
-                }
-            }
+        if let Some(res) = res {
+            self.propagate_item(res);
         }
 
         intravisit::walk_expr(self, expr)
@@ -198,20 +185,22 @@ impl<'tcx> ReachableContext<'tcx> {
                     // Reachable constants will be inlined into other crates
                     // unconditionally, so we need to make sure that their
                     // contents are also reachable.
-                    hir::ItemKind::Const(_, _, init) => {
-                        self.visit_nested_body(init);
+                    hir::ItemKind::Const(..) => {
+                        match self.tcx.const_eval_poly_to_alloc(item.owner_id.def_id.into()) {
+                            Ok(alloc) => {
+                                let alloc = self.tcx.global_alloc(alloc.alloc_id).unwrap_memory();
+                                self.propagate_from_alloc(alloc);
+                            }
+                            Err(ErrorHandled::TooGeneric(span)) => span_bug!(
+                                span,
+                                "generic constants aren't implemented in reachability"
+                            ),
+                            Err(ErrorHandled::Reported(..)) => {}
+                        }
                     }
-
-                    // Reachable statics are inlined if read from another constant or static
-                    // in other crates. Additionally anonymous nested statics may be created
-                    // when evaluating a static, so preserve those, too.
-                    hir::ItemKind::Static(_, _, init) => {
-                        // FIXME(oli-obk): remove this body walking and instead walk the evaluated initializer
-                        // to find nested items that end up in the final value instead of also marking symbols
-                        // as reachable that are only needed for evaluation.
-                        self.visit_nested_body(init);
+                    hir::ItemKind::Static(..) => {
                         if let Ok(alloc) = self.tcx.eval_static_initializer(item.owner_id.def_id) {
-                            self.propagate_statics_from_alloc(item.owner_id.def_id, alloc);
+                            self.propagate_from_alloc(alloc);
                         }
                     }
 
@@ -282,28 +271,89 @@ impl<'tcx> ReachableContext<'tcx> {
         }
     }
 
-    /// Finds anonymous nested statics created for nested allocations and adds them to `reachable_symbols`.
-    fn propagate_statics_from_alloc(&mut self, root: LocalDefId, alloc: ConstAllocation<'tcx>) {
+    /// Finds things to add to `reachable_symbols` within allocations.
+    /// In contrast to visit_nested_body this ignores things that were only needed to evaluate
+    /// the allocation.
+    fn propagate_from_alloc(&mut self, alloc: ConstAllocation<'tcx>) {
         if !self.any_library {
             return;
         }
         for (_, prov) in alloc.0.provenance().ptrs().iter() {
             match self.tcx.global_alloc(prov.alloc_id()) {
                 GlobalAlloc::Static(def_id) => {
-                    if let Some(def_id) = def_id.as_local()
-                        && self.tcx.local_parent(def_id) == root
-                        // This is the main purpose of this function: add the def_id we find
-                        // to `reachable_symbols`.
-                        && self.reachable_symbols.insert(def_id)
-                        && let Ok(alloc) = self.tcx.eval_static_initializer(def_id)
-                    {
-                        self.propagate_statics_from_alloc(root, alloc);
+                    self.propagate_item(Res::Def(self.tcx.def_kind(def_id), def_id))
+                }
+                GlobalAlloc::Function(instance) => {
+                    // Manually visit to actually see the instance's `DefId`. Type visitors won't see it
+                    self.propagate_item(Res::Def(
+                        self.tcx.def_kind(instance.def_id()),
+                        instance.def_id(),
+                    ));
+                    self.visit(instance.args);
+                }
+                GlobalAlloc::VTable(ty, trait_ref) => {
+                    self.visit(ty);
+                    // Manually visit to actually see the trait's `DefId`. Type visitors won't see it
+                    if let Some(trait_ref) = trait_ref {
+                        let ExistentialTraitRef { def_id, args } = trait_ref.skip_binder();
+                        self.visit_def_id(def_id, "", &"");
+                        self.visit(args);
                     }
                 }
-                GlobalAlloc::Function(_) | GlobalAlloc::VTable(_, _) | GlobalAlloc::Memory(_) => {}
+                GlobalAlloc::Memory(alloc) => self.propagate_from_alloc(alloc),
             }
         }
     }
+
+    fn propagate_item(&mut self, res: Res) {
+        let Res::Def(kind, def_id) = res else { return };
+        let Some(def_id) = def_id.as_local() else { return };
+        match kind {
+            DefKind::Static { nested: true, .. } => {
+                // This is the main purpose of this function: add the def_id we find
+                // to `reachable_symbols`.
+                if self.reachable_symbols.insert(def_id) {
+                    if let Ok(alloc) = self.tcx.eval_static_initializer(def_id) {
+                        // This cannot cause infinite recursion, because we abort by inserting into the
+                        // work list once we hit a normal static. Nested statics, even if they somehow
+                        // become recursive, are also not infinitely recursing, because of the
+                        // `reachable_symbols` check above.
+                        // We still need to protect against stack overflow due to deeply nested statics.
+                        ensure_sufficient_stack(|| self.propagate_from_alloc(alloc));
+                    }
+                }
+            }
+            // Reachable constants and reachable statics can have their contents inlined
+            // into other crates. Mark them as reachable and recurse into their body.
+            DefKind::Const | DefKind::AssocConst | DefKind::Static { .. } => {
+                self.worklist.push(def_id);
+            }
+            _ => {
+                if self.def_id_represents_local_inlined_item(def_id.to_def_id()) {
+                    self.worklist.push(def_id);
+                } else {
+                    self.reachable_symbols.insert(def_id);
+                }
+            }
+        }
+    }
+}
+
+impl<'tcx> DefIdVisitor<'tcx> for ReachableContext<'tcx> {
+    type Result = ();
+
+    fn tcx(&self) -> TyCtxt<'tcx> {
+        self.tcx
+    }
+
+    fn visit_def_id(
+        &mut self,
+        def_id: DefId,
+        _kind: &str,
+        _descr: &dyn std::fmt::Display,
+    ) -> Self::Result {
+        self.propagate_item(Res::Def(self.tcx.def_kind(def_id), def_id))
+    }
 }
 
 fn check_item<'tcx>(

compiler/rustc_privacy/src/lib.rs

@@ -67,7 +67,7 @@ impl<'tcx> fmt::Display for LazyDefPathStr<'tcx> {
 /// First, it doesn't have overridable `fn visit_trait_ref`, so we have to catch trait `DefId`s
 /// manually. Second, it doesn't visit some type components like signatures of fn types, or traits
 /// in `impl Trait`, see individual comments in `DefIdVisitorSkeleton::visit_ty`.
-trait DefIdVisitor<'tcx> {
+pub trait DefIdVisitor<'tcx> {
     type Result: VisitorResult = ();
     const SHALLOW: bool = false;
     const SKIP_ASSOC_TYS: bool = false;
@@ -98,7 +98,7 @@ trait DefIdVisitor<'tcx> {
     }
 }
 
-struct DefIdVisitorSkeleton<'v, 'tcx, V: ?Sized> {
+pub struct DefIdVisitorSkeleton<'v, 'tcx, V: ?Sized> {
     def_id_visitor: &'v mut V,
     visited_opaque_tys: FxHashSet<DefId>,
     dummy: PhantomData<TyCtxt<'tcx>>,

tests/codegen/dont_codegen_private_const_fn_only_used_in_const_eval.rs

@@ -0,0 +1,16 @@
+//! This test checks that we do not monomorphize functions that are only
+//! used to evaluate static items, but never used in runtime code.
+
+//@compile-flags: --crate-type=lib -Copt-level=0
+
+const fn foo() {}
+
+pub static FOO: () = foo();
+
+// CHECK-NOT: define{{.*}}foo{{.*}}
+
+const fn bar() {}
+
+pub const BAR: () = bar();
+
+// CHECK-NOT: define{{.*}}bar{{.*}}

tests/ui/cross-crate/auxiliary/static_init_aux.rs

@@ -1,6 +1,8 @@
 pub static V: &u32 = &X;
 pub static F: fn() = f;
 pub static G: fn() = G0;
+pub static H: &(dyn Fn() + Sync) = &h;
+pub static I: fn() = Helper(j).mk();
 
 static X: u32 = 42;
 static G0: fn() = g;
@@ -12,3 +14,22 @@ pub fn v() -> *const u32 {
 fn f() {}
 
 fn g() {}
+
+fn h() {}
+
+#[derive(Copy, Clone)]
+struct Helper<T: Copy>(T);
+
+impl<T: Copy + FnOnce()> Helper<T> {
+    const fn mk(self) -> fn() {
+        i::<T>
+    }
+}
+
+fn i<T: FnOnce()>() {
+    assert_eq!(std::mem::size_of::<T>(), 0);
+    // unsafe to work around the lack of a `Default` impl for function items
+    unsafe { (std::mem::transmute_copy::<(), T>(&()))() }
+}
+
+fn j() {}

tests/ui/cross-crate/static-init.rs

@@ -6,6 +6,8 @@ extern crate static_init_aux as aux;
 static V: &u32 = aux::V;
 static F: fn() = aux::F;
 static G: fn() = aux::G;
+static H: &(dyn Fn() + Sync) = aux::H;
+static I: fn() = aux::I;
 
 fn v() -> *const u32 {
     V
@@ -15,4 +17,6 @@ fn main() {
     assert_eq!(aux::v(), crate::v());
     F();
     G();
+    H();
+    I();
 }