Skip to content

Use less HIR to compute effective visibility. #144554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 2, 2025
+119 −101
Merged

Use less HIR to compute effective visibility. #144554

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
220 changes: 119 additions & 101 deletions compiler/rustc_privacy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ use rustc_errors::{MultiSpan, listify};
use rustc_hir as hir;
use rustc_hir::attrs::AttributeKind;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{CRATE_DEF_ID, DefId, LocalDefId, LocalModDefId};
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
use rustc_hir::intravisit::{self, InferKind, Visitor};
use rustc_hir::{AmbigArg, ForeignItemId, ItemId, PatKind, find_attr};
use rustc_hir::{AmbigArg, ForeignItemId, ItemId, OwnerId, PatKind, find_attr};
use rustc_middle::middle::privacy::{EffectiveVisibilities, EffectiveVisibility, Level};
use rustc_middle::query::Providers;
use rustc_middle::ty::print::PrintTraitRefExt as _;
Expand Down Expand Up @@ -599,7 +599,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {
DefKind::Struct | DefKind::Union => {
// While structs and unions have type privacy, their fields do not.
let struct_def = self.tcx.adt_def(def_id);
for field in struct_def.non_enum_variant().fields.iter() {
for field in &struct_def.non_enum_variant().fields {
let def_id = field.did.expect_local();
let field_vis = self.tcx.local_visibility(def_id);
if field_vis.is_accessible_from(module, self.tcx) {
Expand Down Expand Up @@ -637,58 +637,62 @@ impl<'tcx> EmbargoVisitor<'tcx> {
}
}

impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
impl<'tcx> EmbargoVisitor<'tcx> {
fn check_def_id(&mut self, owner_id: OwnerId) {
// Update levels of nested things and mark all items
// in interfaces of reachable items as reachable.
let item_ev = self.get(item.owner_id.def_id);
match item.kind {
let item_ev = self.get(owner_id.def_id);
match self.tcx.def_kind(owner_id) {
// The interface is empty, and no nested items.
hir::ItemKind::Use(..)
| hir::ItemKind::ExternCrate(..)
| hir::ItemKind::GlobalAsm { .. } => {}
// The interface is empty, and all nested items are processed by `visit_item`.
hir::ItemKind::Mod(..) => {}
hir::ItemKind::Macro(_, macro_def, _) => {
DefKind::Use | DefKind::ExternCrate | DefKind::GlobalAsm => {}
// The interface is empty, and all nested items are processed by `check_def_id`.
DefKind::Mod => {}
DefKind::Macro { .. } => {
if let Some(item_ev) = item_ev {
self.update_reachability_from_macro(item.owner_id.def_id, macro_def, item_ev);
let (_, macro_def, _) =
self.tcx.hir_expect_item(owner_id.def_id).expect_macro();
self.update_reachability_from_macro(owner_id.def_id, macro_def, item_ev);
}
}
hir::ItemKind::Const(..)
| hir::ItemKind::Static(..)
| hir::ItemKind::Fn { .. }
| hir::ItemKind::TyAlias(..) => {
DefKind::ForeignTy
| DefKind::Const
| DefKind::Static { .. }
| DefKind::Fn
| DefKind::TyAlias => {
if let Some(item_ev) = item_ev {
self.reach(item.owner_id.def_id, item_ev).generics().predicates().ty();
self.reach(owner_id.def_id, item_ev).generics().predicates().ty();
}
}
hir::ItemKind::Trait(.., trait_item_refs) => {
DefKind::Trait => {
if let Some(item_ev) = item_ev {
self.reach(item.owner_id.def_id, item_ev).generics().predicates();
self.reach(owner_id.def_id, item_ev).generics().predicates();

for assoc_item in self.tcx.associated_items(owner_id).in_definition_order() {
if assoc_item.is_impl_trait_in_trait() {
continue;
}

for trait_item_ref in trait_item_refs {
self.update(trait_item_ref.owner_id.def_id, item_ev, Level::Reachable);
let def_id = assoc_item.def_id.expect_local();
self.update(def_id, item_ev, Level::Reachable);

let tcx = self.tcx;
let mut reach = self.reach(trait_item_ref.owner_id.def_id, item_ev);
let mut reach = self.reach(def_id, item_ev);
reach.generics().predicates();

if let DefKind::AssocTy = tcx.def_kind(trait_item_ref.owner_id)
&& !tcx.defaultness(trait_item_ref.owner_id).has_value()
{
if assoc_item.is_type() && !assoc_item.defaultness(tcx).has_value() {
// No type to visit.
} else {
reach.ty();
}
}
}
}
hir::ItemKind::TraitAlias(..) => {
DefKind::TraitAlias => {
if let Some(item_ev) = item_ev {
self.reach(item.owner_id.def_id, item_ev).generics().predicates();
self.reach(owner_id.def_id, item_ev).generics().predicates();
}
}
hir::ItemKind::Impl(impl_) => {
DefKind::Impl { of_trait } => {
// Type inference is very smart sometimes. It can make an impl reachable even some
// components of its type or trait are unreachable. E.g. methods of
// `impl ReachableTrait<UnreachableTy> for ReachableTy<UnreachableTy> { ... }`
Expand All @@ -700,85 +704,100 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
// without knowing both "shallow" version of its self type and "shallow" version of
// its trait if it exists (which require reaching the `DefId`s in them).
let item_ev = EffectiveVisibility::of_impl::<true>(
item.owner_id.def_id,
owner_id.def_id,
self.tcx,
&self.effective_visibilities,
);

self.update_eff_vis(item.owner_id.def_id, item_ev, None, Level::Direct);
self.update_eff_vis(owner_id.def_id, item_ev, None, Level::Direct);

self.reach(owner_id.def_id, item_ev).generics().predicates().ty().trait_ref();

self.reach(item.owner_id.def_id, item_ev).generics().predicates().ty().trait_ref();
for assoc_item in self.tcx.associated_items(owner_id).in_definition_order() {
if assoc_item.is_impl_trait_in_trait() {
continue;
}

for impl_item_ref in impl_.items {
let def_id = impl_item_ref.owner_id.def_id;
let def_id = assoc_item.def_id.expect_local();
let max_vis =
impl_.of_trait.is_none().then(|| self.tcx.local_visibility(def_id));
if of_trait { None } else { Some(self.tcx.local_visibility(def_id)) };
self.update_eff_vis(def_id, item_ev, max_vis, Level::Direct);

if let Some(impl_item_ev) = self.get(def_id) {
self.reach(def_id, impl_item_ev).generics().predicates().ty();
}
}
}
hir::ItemKind::Enum(_, _, ref def) => {
DefKind::Enum => {
if let Some(item_ev) = item_ev {
self.reach(item.owner_id.def_id, item_ev).generics().predicates();
self.reach(owner_id.def_id, item_ev).generics().predicates();
}
for variant in def.variants {
let def = self.tcx.adt_def(owner_id);
for variant in def.variants() {
if let Some(item_ev) = item_ev {
self.update(variant.def_id, item_ev, Level::Reachable);
self.update(variant.def_id.expect_local(), item_ev, Level::Reachable);
}

if let Some(variant_ev) = self.get(variant.def_id) {
if let Some(ctor_def_id) = variant.data.ctor_def_id() {
self.update(ctor_def_id, variant_ev, Level::Reachable);
if let Some(variant_ev) = self.get(variant.def_id.expect_local()) {
if let Some(ctor_def_id) = variant.ctor_def_id() {
self.update(ctor_def_id.expect_local(), variant_ev, Level::Reachable);
}

for field in variant.data.fields() {
self.update(field.def_id, variant_ev, Level::Reachable);
self.reach(field.def_id, variant_ev).ty();
for field in &variant.fields {
let field = field.did.expect_local();
self.update(field, variant_ev, Level::Reachable);
self.reach(field, variant_ev).ty();
}
// Corner case: if the variant is reachable, but its
// enum is not, make the enum reachable as well.
self.reach(item.owner_id.def_id, variant_ev).ty();
self.reach(owner_id.def_id, variant_ev).ty();
}
if let Some(ctor_def_id) = variant.data.ctor_def_id() {
if let Some(ctor_ev) = self.get(ctor_def_id) {
self.reach(item.owner_id.def_id, ctor_ev).ty();
if let Some(ctor_def_id) = variant.ctor_def_id() {
if let Some(ctor_ev) = self.get(ctor_def_id.expect_local()) {
self.reach(owner_id.def_id, ctor_ev).ty();
}
}
}
}
hir::ItemKind::ForeignMod { items, .. } => {
for foreign_item in items {
if let Some(foreign_item_ev) = self.get(foreign_item.owner_id.def_id) {
self.reach(foreign_item.owner_id.def_id, foreign_item_ev)
.generics()
.predicates()
.ty();
}
}
}
hir::ItemKind::Struct(_, _, ref struct_def)
| hir::ItemKind::Union(_, _, ref struct_def) => {
DefKind::Struct | DefKind::Union => {
let def = self.tcx.adt_def(owner_id).non_enum_variant();
if let Some(item_ev) = item_ev {
self.reach(item.owner_id.def_id, item_ev).generics().predicates();
for field in struct_def.fields() {
self.update(field.def_id, item_ev, Level::Reachable);
if let Some(field_ev) = self.get(field.def_id) {
self.reach(field.def_id, field_ev).ty();
self.reach(owner_id.def_id, item_ev).generics().predicates();
for field in &def.fields {
let field = field.did.expect_local();
self.update(field, item_ev, Level::Reachable);
if let Some(field_ev) = self.get(field) {
self.reach(field, field_ev).ty();
}
}
}
if let Some(ctor_def_id) = struct_def.ctor_def_id() {
if let Some(ctor_def_id) = def.ctor_def_id() {
if let Some(item_ev) = item_ev {
self.update(ctor_def_id, item_ev, Level::Reachable);
self.update(ctor_def_id.expect_local(), item_ev, Level::Reachable);
}
if let Some(ctor_ev) = self.get(ctor_def_id) {
self.reach(item.owner_id.def_id, ctor_ev).ty();
if let Some(ctor_ev) = self.get(ctor_def_id.expect_local()) {
self.reach(owner_id.def_id, ctor_ev).ty();
}
}
}
// Contents are checked directly.
DefKind::ForeignMod => {}
DefKind::Field
| DefKind::Variant
| DefKind::AssocFn
| DefKind::AssocTy
| DefKind::AssocConst
| DefKind::TyParam
| DefKind::AnonConst
| DefKind::InlineConst
| DefKind::OpaqueTy
| DefKind::Closure
| DefKind::SyntheticCoroutineBody
| DefKind::ConstParam
| DefKind::LifetimeParam
| DefKind::Ctor(..) => {
bug!("should be checked while checking parent")
}
}
}
}
Expand Down Expand Up @@ -839,7 +858,7 @@ pub struct TestReachabilityVisitor<'a, 'tcx> {
}

impl<'a, 'tcx> TestReachabilityVisitor<'a, 'tcx> {
fn effective_visibility_diagnostic(&mut self, def_id: LocalDefId) {
fn effective_visibility_diagnostic(&self, def_id: LocalDefId) {
if self.tcx.has_attr(def_id, sym::rustc_effective_visibility) {
let mut error_msg = String::new();
let span = self.tcx.def_span(def_id.to_def_id());
Expand All @@ -859,43 +878,35 @@ impl<'a, 'tcx> TestReachabilityVisitor<'a, 'tcx> {
}
}

impl<'a, 'tcx> Visitor<'tcx> for TestReachabilityVisitor<'a, 'tcx> {
fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
self.effective_visibility_diagnostic(item.owner_id.def_id);
impl<'a, 'tcx> TestReachabilityVisitor<'a, 'tcx> {
fn check_def_id(&self, owner_id: OwnerId) {
self.effective_visibility_diagnostic(owner_id.def_id);

match item.kind {
hir::ItemKind::Enum(_, _, ref def) => {
for variant in def.variants.iter() {
self.effective_visibility_diagnostic(variant.def_id);
if let Some(ctor_def_id) = variant.data.ctor_def_id() {
self.effective_visibility_diagnostic(ctor_def_id);
match self.tcx.def_kind(owner_id) {
DefKind::Enum => {
let def = self.tcx.adt_def(owner_id.def_id);
for variant in def.variants() {
self.effective_visibility_diagnostic(variant.def_id.expect_local());
if let Some(ctor_def_id) = variant.ctor_def_id() {
self.effective_visibility_diagnostic(ctor_def_id.expect_local());
}
for field in variant.data.fields() {
self.effective_visibility_diagnostic(field.def_id);
for field in &variant.fields {
self.effective_visibility_diagnostic(field.did.expect_local());
}
}
}
hir::ItemKind::Struct(_, _, ref def) | hir::ItemKind::Union(_, _, ref def) => {
DefKind::Struct | DefKind::Union => {
let def = self.tcx.adt_def(owner_id.def_id).non_enum_variant();
if let Some(ctor_def_id) = def.ctor_def_id() {
self.effective_visibility_diagnostic(ctor_def_id);
self.effective_visibility_diagnostic(ctor_def_id.expect_local());
}
for field in def.fields() {
self.effective_visibility_diagnostic(field.def_id);
for field in &def.fields {
self.effective_visibility_diagnostic(field.did.expect_local());
}
}
_ => {}
}
}

fn visit_trait_item(&mut self, item: &'tcx hir::TraitItem<'tcx>) {
self.effective_visibility_diagnostic(item.owner_id.def_id);
}
fn visit_impl_item(&mut self, item: &'tcx hir::ImplItem<'tcx>) {
self.effective_visibility_diagnostic(item.owner_id.def_id);
}
fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem<'tcx>) {
self.effective_visibility_diagnostic(item.owner_id.def_id);
}
}

//////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1836,8 +1847,14 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities {
visitor.changed = false;
}

let crate_items = tcx.hir_crate_items(());
loop {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR does maybe half of work required to turn this loop into a work queue of OwnerIds (to avoid visiting the crate repeatedly until the algorithm saturates).

tcx.hir_visit_all_item_likes_in_crate(&mut visitor);
for id in crate_items.free_items() {
visitor.check_def_id(id.owner_id);
}
for id in crate_items.foreign_items() {
visitor.check_def_id(id.owner_id);
}
if visitor.changed {
visitor.changed = false;
} else {
Expand All @@ -1846,10 +1863,11 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities {
}
visitor.effective_visibilities.check_invariants(tcx);

let mut check_visitor =
let check_visitor =
TestReachabilityVisitor { tcx, effective_visibilities: &visitor.effective_visibilities };
check_visitor.effective_visibility_diagnostic(CRATE_DEF_ID);
tcx.hir_visit_all_item_likes_in_crate(&mut check_visitor);
for id in crate_items.owners() {
check_visitor.check_def_id(id);
}

tcx.arena.alloc(visitor.effective_visibilities)
}
Expand Down
Loading