Skip to content

Commit

Permalink
Auto merge of rust-lang#122371 - oli-obk:visit_nested_body, r=<try>
Browse files Browse the repository at this point in the history
Stop walking the bodies of statics for reachability, and evaluate them instead

cc `@saethlin` `@RalfJung`

cc rust-lang#119214

This reuses the `DefIdVisitor` from `rustc_privacy`, because they basically try to do the same thing. It can probably be extended to constants, too, but let's tackle that separately, it's likely more involved.
  • Loading branch information
bors committed Mar 12, 2024
2 parents b0170b6 + 7b7b6b5 commit 1a609b9
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 21 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Expand Up @@ -4400,6 +4400,7 @@ dependencies = [
"rustc_lexer",
"rustc_macros",
"rustc_middle",
"rustc_privacy",
"rustc_session",
"rustc_span",
"rustc_target",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_passes/Cargo.toml
Expand Up @@ -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" }
Expand Down
100 changes: 81 additions & 19 deletions compiler/rustc_passes/src/reachable.rs
Expand Up @@ -13,8 +13,10 @@ 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::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;

Expand Down Expand Up @@ -64,23 +66,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)
Expand Down Expand Up @@ -197,9 +184,14 @@ 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) | hir::ItemKind::Static(_, _, init) => {
hir::ItemKind::Const(_, _, init) => {
self.visit_nested_body(init);
}
hir::ItemKind::Static(..) => {
if let Ok(alloc) = self.tcx.eval_static_initializer(item.owner_id.def_id) {
self.propagate_from_alloc(item.owner_id.def_id, alloc);
}
}

// These are normal, nothing reachable about these
// inherently and their children are already in the
Expand Down Expand Up @@ -266,6 +258,76 @@ impl<'tcx> ReachableContext<'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
/// to the allocation.
fn propagate_from_alloc(&mut self, root: LocalDefId, 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) => {
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::Memory(alloc) => self.propagate_from_alloc(root, 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 };
if self.def_id_represents_local_inlined_item(def_id.to_def_id()) {
self.worklist.push(def_id);
} else {
match kind {
// 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);
}
_ => {
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>(
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_privacy/src/lib.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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>>,
Expand Down
21 changes: 21 additions & 0 deletions 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;
Expand All @@ -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() {}
4 changes: 4 additions & 0 deletions tests/ui/cross-crate/static-init.rs
Expand Up @@ -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
Expand All @@ -15,4 +17,6 @@ fn main() {
assert_eq!(aux::v(), crate::v());
F();
G();
H();
I();
}

0 comments on commit 1a609b9

Please sign in to comment.