Privacy: try use queue instead of fixed-point iteration by Bryanskiy · Pull Request #156228 · rust-lang/rust

Bryanskiy · 2026-05-06T09:54:42Z

Earlier: iterate until updates of EffectiveVisibility occur.

Now: if an update occurs, then we put in the queue those items that may be affected by this update. Iterate until there are items in the queue.

r? @petrochenkov

petrochenkov · 2026-05-06T10:02:24Z

@bors try @rust-timer queue

Privacy: try use queue instead of fixed-point iteration

rust-bors · 2026-05-06T12:10:56Z

☀️ Try build successful (CI)
Build commit: cf797a3 (cf797a383bca844619c7f399ee6940b7723eab30, parent: e95e73209faf6ead2bc5c7636e45e589a751b79b)

rust-timer · 2026-05-06T12:52:57Z

Finished benchmarking commit (cf797a3): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.8%	[-5.3%, -0.1%]	101
Improvements ✅ (secondary)	-0.3%	[-1.0%, -0.1%]	17
All ❌✅ (primary)	-0.8%	[-5.3%, -0.1%]	101

Max RSS (memory usage)

Results (primary -1.9%, secondary -0.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.0%	[2.6%, 3.3%]	2
Improvements ✅ (primary)	-1.9%	[-1.9%, -1.9%]	1
Improvements ✅ (secondary)	-1.9%	[-2.5%, -0.4%]	5
All ❌✅ (primary)	-1.9%	[-1.9%, -1.9%]	1

Cycles

Results (primary -0.1%, secondary -0.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.8%	[2.8%, 2.8%]	1
Regressions ❌ (secondary)	1.8%	[0.4%, 5.0%]	4
Improvements ✅ (primary)	-3.0%	[-3.0%, -3.0%]	1
Improvements ✅ (secondary)	-1.9%	[-4.5%, -0.4%]	5
All ❌✅ (primary)	-0.1%	[-3.0%, 2.8%]	2

Binary size

Results (primary 0.1%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.1%]	31
Regressions ❌ (secondary)	0.1%	[0.0%, 0.1%]	21
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.1%]	31

Bootstrap: 493.455s -> 502.405s (1.81%)
Artifact size: 394.43 MiB -> 394.45 MiB (0.00%)

petrochenkov · 2026-05-12T10:59:49Z

Blocked on #156500.
@rustbot blocked

@petrochenkov

Privacy: move macros handling to early stage The patch moves effective visibility computation for macros from `rustc_privacy` to `rustc_resolve`. It will enable this optimization: rust-lang#156228. However, I found some problems with macro handling while I was doing this. The current implementation was written ~6 years ago and checks the reachability of a definition from a macro by nominal visibility. In general this is incorrect. For example, in the current implementation modules are not traversed if their nominal visibility is less then the nominal visibility of a module defining macro: https://github.com/rust-lang/rust/blob/29b7590130c83542a095cdf1323ed0f78eec2bb8/compiler/rustc_privacy/src/lib.rs#L618-L626 As a result, in order to compile code like `tests/ui/definition-reachable/auxiliary/field-method-macro.rs`. we have to additionally traverse types of adt fields: https://github.com/rust-lang/rust/blob/29b7590130c83542a095cdf1323ed0f78eec2bb8/compiler/rustc_privacy/src/lib.rs#L628-L638 This is a hack and the proper solution would be to check definitions with `EffectiveVisibilities::is_reachable`. I haven’t done this yet, as it would start to trigger many lints as more items become reachable. I think it’s better to leave the change to another commit. r? @petrochenkov

@petrochenkov

Privacy: move macros handling to early stage The patch moves effective visibility computation for macros from `rustc_privacy` to `rustc_resolve`. It will enable this optimization: rust-lang#156228. However, I found some problems with macro handling while I was doing this. The current implementation was written ~6 years ago and checks the reachability of a definition from a macro by nominal visibility. In general this is incorrect. For example, in the current implementation modules are not traversed if their nominal visibility is less then the nominal visibility of a module defining macro: https://github.com/rust-lang/rust/blob/29b7590130c83542a095cdf1323ed0f78eec2bb8/compiler/rustc_privacy/src/lib.rs#L618-L626 As a result, in order to compile code like `tests/ui/definition-reachable/auxiliary/field-method-macro.rs`. we have to additionally traverse types of adt fields: https://github.com/rust-lang/rust/blob/29b7590130c83542a095cdf1323ed0f78eec2bb8/compiler/rustc_privacy/src/lib.rs#L628-L638 This is a hack and the proper solution would be to check definitions with `EffectiveVisibilities::is_reachable`. I haven’t done this yet, as it would start to trigger many lints as more items become reachable. I think it’s better to leave the change to another commit. r? @petrochenkov

@petrochenkov

Privacy: move macros handling to early stage The patch moves effective visibility computation for macros from `rustc_privacy` to `rustc_resolve`. It will enable this optimization: rust-lang#156228. However, I found some problems with macro handling while I was doing this. The current implementation was written ~6 years ago and checks the reachability of a definition from a macro by nominal visibility. In general this is incorrect. For example, in the current implementation modules are not traversed if their nominal visibility is less then the nominal visibility of a module defining macro: https://github.com/rust-lang/rust/blob/29b7590130c83542a095cdf1323ed0f78eec2bb8/compiler/rustc_privacy/src/lib.rs#L618-L626 As a result, in order to compile code like `tests/ui/definition-reachable/auxiliary/field-method-macro.rs`. we have to additionally traverse types of adt fields: https://github.com/rust-lang/rust/blob/29b7590130c83542a095cdf1323ed0f78eec2bb8/compiler/rustc_privacy/src/lib.rs#L628-L638 This is a hack and the proper solution would be to check definitions with `EffectiveVisibilities::is_reachable`. I haven’t done this yet, as it would start to trigger many lints as more items become reachable. I think it’s better to leave the change to another commit. r? @petrochenkov

@petrochenkov

Rollup merge of #156500 - Bryanskiy:macros_vis, r=petrochenkov Privacy: move macros handling to early stage The patch moves effective visibility computation for macros from `rustc_privacy` to `rustc_resolve`. It will enable this optimization: #156228. However, I found some problems with macro handling while I was doing this. The current implementation was written ~6 years ago and checks the reachability of a definition from a macro by nominal visibility. In general this is incorrect. For example, in the current implementation modules are not traversed if their nominal visibility is less then the nominal visibility of a module defining macro: https://github.com/rust-lang/rust/blob/29b7590130c83542a095cdf1323ed0f78eec2bb8/compiler/rustc_privacy/src/lib.rs#L618-L626 As a result, in order to compile code like `tests/ui/definition-reachable/auxiliary/field-method-macro.rs`. we have to additionally traverse types of adt fields: https://github.com/rust-lang/rust/blob/29b7590130c83542a095cdf1323ed0f78eec2bb8/compiler/rustc_privacy/src/lib.rs#L628-L638 This is a hack and the proper solution would be to check definitions with `EffectiveVisibilities::is_reachable`. I haven’t done this yet, as it would start to trigger many lints as more items become reachable. I think it’s better to leave the change to another commit. r? @petrochenkov

petrochenkov · 2026-05-14T09:25:42Z

@bors try @rust-timer queue

Privacy: try use queue instead of fixed-point iteration

rust-bors · 2026-05-14T11:35:01Z

☀️ Try build successful (CI)
Build commit: 4d80111 (4d801117ba4a06e4f29235e7151351a9ea0a66df, parent: 1a70f8d36e818b45829a5c065fd6004c9047e8d1)

rust-timer · 2026-05-14T12:15:41Z

Finished benchmarking commit (4d80111): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	3
Improvements ✅ (primary)	-0.9%	[-6.4%, -0.1%]	133
Improvements ✅ (secondary)	-0.4%	[-1.3%, -0.1%]	29
All ❌✅ (primary)	-0.9%	[-6.4%, -0.1%]	133

Max RSS (memory usage)

Results (secondary -2.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -9.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.3%	[2.3%, 2.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-10.5%	[-14.9%, -6.6%]	8
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 512.682s -> 510.327s (-0.46%)
Artifact size: 398.03 MiB -> 398.07 MiB (0.01%)

petrochenkov · 2026-05-14T14:49:41Z

+            | DefKind::Use
+            | DefKind::ExternCrate
+            | DefKind::GlobalAsm
+            | DefKind::Ctor(..) => {}


Could you mark unreachable cases with bug!(), like in check_def_id?

View changes since the review

petrochenkov · 2026-05-14T15:06:52Z

        if let Some(def_id) = def_id.as_local() {
+            if matches!(self.tcx().def_kind(self.item_def_id), DefKind::Impl { .. })
+                && matches!(
+                    self.tcx().def_kind(def_id),


This condition is unnecessary and error-prone, any def-id that we encounter here will affect the impl's visibility.
Similarly, the type_to_impls.get(...) check needs to be performed for any reachable DefKind as well.

I think we can temporary add some asserts to figure out which DefKinds actually end up being added to this set.

View changes since the review

petrochenkov · 2026-05-14T15:08:36Z

    }
 }

 impl<'tcx> DefIdVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'_, 'tcx> {


This is not the best place to determine the set of def-ids affecting impl's effvis.
It's correct, but too conservative.
The right set of def-ids is the one that is visited by EffectiveVisibility::of_impl::<true>.

View changes since the review

petrochenkov · 2026-05-14T15:16:10Z

+            | DefKind::ForeignTy
+            | DefKind::TyAlias => {
+                self.queue.push(def_id);
+                if let Some(impls) = self.type_to_impls.get(&def_id) {


Given #156228 (comment), I think it would be safest to perform these two actions (add self + add impls) for all reachable DefKinds.

View changes since the review

petrochenkov · 2026-05-14T15:48:43Z

+                self.ev
+                    .type_to_impls
+                    .entry(def_id)
+                    .or_insert_with(Default::default)


Suggested change

.or_insert_with(Default::default)

.or_default()

View changes since the review

rustbot assigned petrochenkov May 6, 2026

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 6, 2026