Skip to content

Commit 60c6dc0

Browse files
committed
resolve: Restore some effective visibility optimizations
Something similar was previously removed as a part of #104602, but after this PR all table changes should also be "locally correct" after every update.
1 parent 22a7a19 commit 60c6dc0

File tree

3 files changed

+34
-14
lines changed

3 files changed

+34
-14
lines changed

Diff for: compiler/rustc_resolve/src/effective_visibilities.rs

+28-8
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,6 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
155155
}
156156
}
157157

158-
fn cheap_private_vis(&self, parent_id: ParentId<'_>) -> Option<Visibility> {
159-
matches!(parent_id, ParentId::Def(_)).then_some(self.current_private_vis)
160-
}
161-
162158
fn effective_vis_or_private(&mut self, parent_id: ParentId<'a>) -> EffectiveVisibility {
163159
// Private nodes are only added to the table for caching, they could be added or removed at
164160
// any moment without consequences, so we don't set `changed` to true when adding them.
@@ -172,29 +168,53 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
172168
}
173169
}
174170

171+
/// All effective visibilities for a node are larger or equal than private visibility
172+
/// for that node (see `check_invariants` in middle/privacy.rs).
173+
/// So if either parent or nominal visibility is the same as private visibility, then
174+
/// `min(parent_vis, nominal_vis) <= private_vis`, and the update logic is guaranteed
175+
/// to not update anything and we can skip it.
176+
///
177+
/// We are checking this condition only if the correct value of private visibility is
178+
/// cheaply available, otherwise it does't make sense performance-wise.
179+
///
180+
/// `None` is returned if the update can be skipped,
181+
/// and cheap private visibility is returned otherwise.
182+
fn may_update(
183+
&self,
184+
nominal_vis: Visibility,
185+
parent_id: ParentId<'_>,
186+
) -> Option<Option<Visibility>> {
187+
match parent_id {
188+
ParentId::Def(def_id) => (nominal_vis != self.current_private_vis
189+
&& self.r.visibilities[&def_id] != self.current_private_vis)
190+
.then_some(Some(self.current_private_vis)),
191+
ParentId::Import(_) => Some(None),
192+
}
193+
}
194+
175195
fn update_import(&mut self, binding: ImportId<'a>, parent_id: ParentId<'a>) {
176196
let nominal_vis = binding.vis.expect_local();
177-
let private_vis = self.cheap_private_vis(parent_id);
197+
let Some(cheap_private_vis) = self.may_update(nominal_vis, parent_id) else { return };
178198
let inherited_eff_vis = self.effective_vis_or_private(parent_id);
179199
let tcx = self.r.tcx;
180200
self.changed |= self.import_effective_visibilities.update(
181201
binding,
182202
nominal_vis,
183-
|| private_vis.unwrap_or_else(|| self.r.private_vis_import(binding)),
203+
|| cheap_private_vis.unwrap_or_else(|| self.r.private_vis_import(binding)),
184204
inherited_eff_vis,
185205
parent_id.level(),
186206
tcx,
187207
);
188208
}
189209

190210
fn update_def(&mut self, def_id: LocalDefId, nominal_vis: Visibility, parent_id: ParentId<'a>) {
191-
let private_vis = self.cheap_private_vis(parent_id);
211+
let Some(cheap_private_vis) = self.may_update(nominal_vis, parent_id) else { return };
192212
let inherited_eff_vis = self.effective_vis_or_private(parent_id);
193213
let tcx = self.r.tcx;
194214
self.changed |= self.def_effective_visibilities.update(
195215
def_id,
196216
nominal_vis,
197-
|| private_vis.unwrap_or_else(|| self.r.private_vis_def(def_id)),
217+
|| cheap_private_vis.unwrap_or_else(|| self.r.private_vis_def(def_id)),
198218
inherited_eff_vis,
199219
parent_id.level(),
200220
tcx,

Diff for: tests/ui/privacy/effective_visibilities.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ mod outer { //~ ERROR Direct: pub(crate), Reexported: pub(crate), Reachable: pub
1818
}
1919

2020
#[rustc_effective_visibility]
21-
struct PrivStruct; //~ ERROR Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
22-
//~| ERROR Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
21+
struct PrivStruct; //~ ERROR not in the table
22+
//~| ERROR not in the table
2323

2424
#[rustc_effective_visibility]
2525
pub union PubUnion { //~ ERROR Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
2626
#[rustc_effective_visibility]
27-
a: u8, //~ ERROR Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
27+
a: u8, //~ ERROR not in the table
2828
#[rustc_effective_visibility]
2929
pub b: u8, //~ ERROR Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
3030
}

Diff for: tests/ui/privacy/effective_visibilities.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,13 @@ error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImpl
3434
LL | pub trait PubTrait {
3535
| ^^^^^^^^^^^^^^^^^^
3636

37-
error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
37+
error: not in the table
3838
--> $DIR/effective_visibilities.rs:21:9
3939
|
4040
LL | struct PrivStruct;
4141
| ^^^^^^^^^^^^^^^^^
4242

43-
error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
43+
error: not in the table
4444
--> $DIR/effective_visibilities.rs:21:9
4545
|
4646
LL | struct PrivStruct;
@@ -52,7 +52,7 @@ error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImpl
5252
LL | pub union PubUnion {
5353
| ^^^^^^^^^^^^^^^^^^
5454

55-
error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
55+
error: not in the table
5656
--> $DIR/effective_visibilities.rs:27:13
5757
|
5858
LL | a: u8,

0 commit comments

Comments
 (0)