Skip to content

Privacy: do not enqueue traits/impls #156755

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

Closed
Bryanskiy wants to merge 1 commit into from
+38 −22
Closed

Privacy: do not enqueue traits/impls #156755

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
60 changes: 38 additions & 22 deletions compiler/rustc_privacy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,8 +560,31 @@ impl<'tcx> EmbargoVisitor<'tcx> {
}

impl<'tcx> EmbargoVisitor<'tcx> {
fn check_assoc_item(&mut self, item: &ty::AssocItem, item_ev: EffectiveVisibility) {
fn check_assoc_item(&mut self, item: &ty::AssocItem, parent_ev: EffectiveVisibility) {
let def_id = item.def_id.expect_local();

let item_ev = match item.container {
AssocContainer::Trait => {
self.update(def_id, parent_ev, Level::Reachable);
parent_ev
}
AssocContainer::InherentImpl => {
self.update_eff_vis(
def_id,
parent_ev,
Some(self.tcx.local_visibility(def_id)),
Level::Direct,
);
let Some(item_ev) = self.get(def_id) else { return };
item_ev
}
AssocContainer::TraitImpl(_) => {
self.update_eff_vis(def_id, parent_ev, None, Level::Direct);
let Some(item_ev) = self.get(def_id) else { return };
item_ev
}
};

let tcx = self.tcx;
let mut reach = self.reach(def_id, item_ev);
reach.generics().predicates();
Expand Down Expand Up @@ -599,9 +622,6 @@ impl<'tcx> EmbargoVisitor<'tcx> {
self.reach(def_id, item_ev).generics().predicates();

for assoc_item in self.tcx.associated_items(def_id).in_definition_order() {
let def_id = assoc_item.def_id.expect_local();
self.update(def_id, item_ev, Level::Reachable);

self.check_assoc_item(assoc_item, item_ev);
}
}
Expand Down Expand Up @@ -640,14 +660,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {
}

for assoc_item in self.tcx.associated_items(def_id).in_definition_order() {
let def_id = assoc_item.def_id.expect_local();
let max_vis =
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.check_assoc_item(assoc_item, impl_item_ev);
}
self.check_assoc_item(assoc_item, item_ev);
}
}
DefKind::Enum => {
Expand Down Expand Up @@ -702,13 +715,18 @@ impl<'tcx> EmbargoVisitor<'tcx> {
}
}
}

DefKind::AssocFn | DefKind::AssocTy | DefKind::AssocConst { .. } => {
Copy link
Copy Markdown
Contributor

@petrochenkov petrochenkov May 19, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Could you add a comment saying that we get here only from the enqueued def-ids?

View changes since the review

let assoc_item = self.tcx.associated_item(def_id);
Copy link
Copy Markdown
Contributor

@petrochenkov petrochenkov May 19, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

This can be moved inside the if below.

View changes since the review

if let Some(parent_ev) = self.get(self.tcx.local_parent(def_id)) {
self.check_assoc_item(&assoc_item, parent_ev);
}
}

// Contents are checked directly.
DefKind::ForeignMod => {}
DefKind::Field
| DefKind::Variant
| DefKind::AssocFn
| DefKind::AssocTy
| DefKind::AssocConst { .. }
| DefKind::TyParam
| DefKind::AnonConst
| DefKind::InlineConst
Expand Down Expand Up @@ -783,16 +801,14 @@ impl ReachEverythingInTheInterfaceVisitor<'_, '_> {
}
}

DefKind::TraitAlias | DefKind::Fn => {
DefKind::TraitAlias
| DefKind::Fn
| DefKind::AssocConst { .. }
| DefKind::AssocFn
| DefKind::AssocTy => {
self.ev.queue.insert(def_id);
}

DefKind::AssocConst { .. } | DefKind::AssocFn | DefKind::AssocTy => {
// FIXME: `EmbargoVisitor` can't check assoc items(see `check_def_id`).
Copy link
Copy Markdown
Contributor

@petrochenkov petrochenkov May 19, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

The FIXME should be removed if we are not going to do this.

View changes since the review

// Let's traverse the whole impl/trait.
self.ev.queue.insert(self.ev.tcx.local_parent(def_id));
}

DefKind::Ctor(ctor_of, _) => {
let update_id = match ctor_of {
CtorOf::Struct => self.ev.tcx.local_parent(def_id),
Expand Down
Loading