Skip to content

Commit a9447bb

Browse files
committed
Perf improvements for effective visibility calculating
1 parent 56f1325 commit a9447bb

File tree

5 files changed

+57
-53
lines changed

5 files changed

+57
-53
lines changed

compiler/rustc_middle/src/middle/privacy.rs

+35-33
Original file line numberDiff line numberDiff line change
@@ -69,21 +69,7 @@ impl EffectiveVisibility {
6969
self.get(tag).is_public()
7070
}
7171

72-
fn update(&mut self, vis: Visibility, tag: AccessLevel, tree: impl DefIdTree) -> bool {
73-
let mut changed = false;
74-
for level in AccessLevel::all_levels() {
75-
if level <= tag {
76-
let current_effective_vis = self.get_mut(level);
77-
if *current_effective_vis != vis && vis.is_at_least(*current_effective_vis, tree) {
78-
changed = true;
79-
*current_effective_vis = vis;
80-
}
81-
}
82-
}
83-
changed
84-
}
85-
86-
fn from_vis(vis: Visibility) -> EffectiveVisibility {
72+
pub fn from_vis(vis: Visibility) -> EffectiveVisibility {
8773
EffectiveVisibility {
8874
public: vis,
8975
exported: vis,
@@ -173,33 +159,49 @@ impl<Id: Hash + Eq + Copy + Into<DefId>> AccessLevels<Id> {
173159
parent_id: Id,
174160
tag: AccessLevel,
175161
tree: impl DefIdTree,
176-
) -> Result<bool, ()> {
162+
) -> bool {
177163
let mut changed = false;
178-
let mut current_effective_vis = self
179-
.get_effective_vis(id)
180-
.copied()
181-
.unwrap_or_else(|| EffectiveVisibility::from_vis(default_vis()));
164+
let mut current_effective_vis = self.get_effective_vis(id).copied().unwrap_or_else(|| {
165+
if id.into().is_crate_root() {
166+
EffectiveVisibility::from_vis(Visibility::Public)
167+
} else {
168+
EffectiveVisibility::from_vis(default_vis())
169+
}
170+
});
182171
if let Some(inherited_effective_vis) = self.get_effective_vis(parent_id) {
172+
let mut inherited_effective_vis_at_prev_level = *inherited_effective_vis.get(tag);
173+
let mut calculated_effective_vis = inherited_effective_vis_at_prev_level;
183174
for level in AccessLevel::all_levels() {
184175
if tag >= level {
185176
let inherited_effective_vis_at_level = *inherited_effective_vis.get(level);
186-
let calculated_effective_vis =
187-
if nominal_vis.is_at_least(inherited_effective_vis_at_level, tree) {
188-
inherited_effective_vis_at_level
189-
} else {
190-
nominal_vis
191-
};
192-
changed |= current_effective_vis.update(calculated_effective_vis, level, tree);
177+
let current_effective_vis_at_level = current_effective_vis.get_mut(level);
178+
// effective visibility for id shouldn't be recalculated if
179+
// inherited from parent_id effective visibility isn't changed at next level
180+
if !(inherited_effective_vis_at_prev_level == inherited_effective_vis_at_level
181+
&& tag != level)
182+
{
183+
calculated_effective_vis =
184+
if nominal_vis.is_at_least(inherited_effective_vis_at_level, tree) {
185+
inherited_effective_vis_at_level
186+
} else {
187+
nominal_vis
188+
};
189+
}
190+
// effective visibility can't be decreased at next update call for the
191+
// same id
192+
if *current_effective_vis_at_level != calculated_effective_vis
193+
&& calculated_effective_vis
194+
.is_at_least(*current_effective_vis_at_level, tree)
195+
{
196+
changed = true;
197+
*current_effective_vis_at_level = calculated_effective_vis;
198+
}
199+
inherited_effective_vis_at_prev_level = inherited_effective_vis_at_level;
193200
}
194201
}
195-
} else {
196-
if !id.into().is_crate_root() {
197-
return Err(());
198-
}
199-
changed |= current_effective_vis.update(Visibility::Public, AccessLevel::Public, tree);
200202
}
201203
self.map.insert(id, current_effective_vis);
202-
Ok(changed)
204+
changed
203205
}
204206
}
205207

compiler/rustc_privacy/src/lib.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -922,9 +922,9 @@ pub struct TestReachabilityVisitor<'tcx, 'a> {
922922
impl<'tcx, 'a> TestReachabilityVisitor<'tcx, 'a> {
923923
fn access_level_diagnostic(&mut self, def_id: LocalDefId) {
924924
if self.tcx.has_attr(def_id.to_def_id(), sym::rustc_effective_visibility) {
925+
let mut error_msg = String::new();
926+
let span = self.tcx.def_span(def_id.to_def_id());
925927
if let Some(effective_vis) = self.access_levels.get_effective_vis(def_id) {
926-
let mut error_msg = String::new();
927-
let span = self.tcx.def_span(def_id.to_def_id());
928928
for level in AccessLevel::all_levels() {
929929
let vis_str = match effective_vis.get(level) {
930930
ty::Visibility::Restricted(restricted_id) => {
@@ -943,8 +943,10 @@ impl<'tcx, 'a> TestReachabilityVisitor<'tcx, 'a> {
943943
}
944944
error_msg.push_str(&format!("{:?}: {}", level, vis_str));
945945
}
946-
self.tcx.sess.emit_err(ReportEffectiveVisibility { span, descr: error_msg });
946+
} else {
947+
error_msg.push_str("not in the table");
947948
}
949+
self.tcx.sess.emit_err(ReportEffectiveVisibility { span, descr: error_msg });
948950
}
949951
}
950952
}

compiler/rustc_resolve/src/access_levels.rs

+11-11
Original file line numberDiff line numberDiff line change
@@ -96,25 +96,25 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
9696
parent_id: LocalDefId,
9797
tag: AccessLevel,
9898
) {
99+
let module_id = self
100+
.r
101+
.get_nearest_non_block_module(def_id.to_def_id())
102+
.nearest_parent_mod()
103+
.expect_local();
104+
if nominal_vis == Visibility::Restricted(module_id)
105+
|| self.r.visibilities[&parent_id] == Visibility::Restricted(module_id)
106+
{
107+
return;
108+
}
99109
let mut access_levels = std::mem::take(&mut self.r.access_levels);
100-
let module_id =
101-
self.r.get_nearest_non_block_module(def_id.to_def_id()).def_id().expect_local();
102-
let res = access_levels.update(
110+
self.changed |= access_levels.update(
103111
def_id,
104112
nominal_vis,
105113
|| Visibility::Restricted(module_id),
106114
parent_id,
107115
tag,
108116
&*self.r,
109117
);
110-
if let Ok(changed) = res {
111-
self.changed |= changed;
112-
} else {
113-
self.r.session.delay_span_bug(
114-
self.r.opt_span(def_id.to_def_id()).unwrap(),
115-
"Can't update effective visibility",
116-
);
117-
}
118118
self.r.access_levels = access_levels;
119119
}
120120
}

src/test/ui/privacy/access_levels.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ mod outer { //~ ERROR Public: pub(crate), Exported: pub(crate), Reachable: pub(c
1717
}
1818

1919
#[rustc_effective_visibility]
20-
struct PrivStruct; //~ ERROR Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
20+
struct PrivStruct; //~ ERROR not in the table
2121

2222
#[rustc_effective_visibility]
2323
pub union PubUnion { //~ ERROR Public: pub(crate), Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
2424
#[rustc_effective_visibility]
25-
a: u8, //~ ERROR Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
25+
a: u8, //~ ERROR not in the table
2626
#[rustc_effective_visibility]
2727
pub b: u8, //~ ERROR Public: pub(crate), Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
2828
}
@@ -38,13 +38,13 @@ mod outer { //~ ERROR Public: pub(crate), Exported: pub(crate), Reachable: pub(c
3838
}
3939

4040
#[rustc_effective_visibility]
41-
macro_rules! none_macro { //~ Public: pub(crate), Exported: pub(crate), Reachable: pub(crate), ReachableFromImplTrait: pub(crate)
41+
macro_rules! none_macro { //~ ERROR Public: pub(crate), Exported: pub(crate), Reachable: pub(crate), ReachableFromImplTrait: pub(crate)
4242
() => {};
4343
}
4444

4545
#[macro_export]
4646
#[rustc_effective_visibility]
47-
macro_rules! public_macro { //~ Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
47+
macro_rules! public_macro { //~ ERROR Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
4848
() => {};
4949
}
5050

src/test/ui/privacy/access_levels.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ error: Public: pub(crate), Exported: pub, Reachable: pub, ReachableFromImplTrait
2222
LL | pub trait PubTrait {
2323
| ^^^^^^^^^^^^^^^^^^
2424

25-
error: Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
25+
error: not in the table
2626
--> $DIR/access_levels.rs:20:9
2727
|
2828
LL | struct PrivStruct;
@@ -34,7 +34,7 @@ error: Public: pub(crate), Exported: pub, Reachable: pub, ReachableFromImplTrait
3434
LL | pub union PubUnion {
3535
| ^^^^^^^^^^^^^^^^^^
3636

37-
error: Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
37+
error: not in the table
3838
--> $DIR/access_levels.rs:25:13
3939
|
4040
LL | a: u8,

0 commit comments

Comments
 (0)