Perf improvements for effective visibility calculating

compiler/rustc_middle/src/middle/privacy.rs

@@ -69,21 +69,7 @@ impl EffectiveVisibility {
         self.get(tag).is_public()
     }
 
-    fn update(&mut self, vis: Visibility, tag: AccessLevel, tree: impl DefIdTree) -> bool {
-        let mut changed = false;
-        for level in AccessLevel::all_levels() {
-            if level <= tag {
-                let current_effective_vis = self.get_mut(level);
-                if *current_effective_vis != vis && vis.is_at_least(*current_effective_vis, tree) {
-                    changed = true;
-                    *current_effective_vis = vis;
-                }
-            }
-        }
-        changed
-    }
-
-    fn from_vis(vis: Visibility) -> EffectiveVisibility {
+    pub fn from_vis(vis: Visibility) -> EffectiveVisibility {
         EffectiveVisibility {
             public: vis,
             exported: vis,
@@ -173,33 +159,49 @@ impl<Id: Hash + Eq + Copy + Into<DefId>> AccessLevels<Id> {
         parent_id: Id,
         tag: AccessLevel,
         tree: impl DefIdTree,
-    ) -> Result<bool, ()> {
+    ) -> bool {
         let mut changed = false;
-        let mut current_effective_vis = self
-            .get_effective_vis(id)
-            .copied()
-            .unwrap_or_else(|| EffectiveVisibility::from_vis(default_vis()));
+        let mut current_effective_vis = self.get_effective_vis(id).copied().unwrap_or_else(|| {
+            if id.into().is_crate_root() {
+                EffectiveVisibility::from_vis(Visibility::Public)
+            } else {
+                EffectiveVisibility::from_vis(default_vis())
+            }
+        });
         if let Some(inherited_effective_vis) = self.get_effective_vis(parent_id) {
+            let mut inherited_effective_vis_at_prev_level = *inherited_effective_vis.get(tag);
+            let mut calculated_effective_vis = inherited_effective_vis_at_prev_level;
             for level in AccessLevel::all_levels() {
                 if tag >= level {
                     let inherited_effective_vis_at_level = *inherited_effective_vis.get(level);
-                    let calculated_effective_vis =
-                        if nominal_vis.is_at_least(inherited_effective_vis_at_level, tree) {
-                            inherited_effective_vis_at_level
-                        } else {
-                            nominal_vis
-                        };
-                    changed |= current_effective_vis.update(calculated_effective_vis, level, tree);
+                    let current_effective_vis_at_level = current_effective_vis.get_mut(level);
+                    // effective visibility for id shouldn't be recalculated if
+                    // inherited from parent_id effective visibility isn't changed at next level
+                    if !(inherited_effective_vis_at_prev_level == inherited_effective_vis_at_level
+                        && tag != level)
+                    {
+                        calculated_effective_vis =
+                            if nominal_vis.is_at_least(inherited_effective_vis_at_level, tree) {
+                                inherited_effective_vis_at_level
+                            } else {
+                                nominal_vis
+                            };
+                    }
+                    // effective visibility can't be decreased at next update call for the
+                    // same id
+                    if *current_effective_vis_at_level != calculated_effective_vis
+                        && calculated_effective_vis
+                            .is_at_least(*current_effective_vis_at_level, tree)
+                    {
+                        changed = true;
+                        *current_effective_vis_at_level = calculated_effective_vis;
+                    }
+                    inherited_effective_vis_at_prev_level = inherited_effective_vis_at_level;
                 }
             }
-        } else {
-            if !id.into().is_crate_root() {
-                return Err(());
-            }
-            changed |= current_effective_vis.update(Visibility::Public, AccessLevel::Public, tree);
         }
         self.map.insert(id, current_effective_vis);
-        Ok(changed)
+        changed
     }
 }
 

compiler/rustc_privacy/src/lib.rs

@@ -922,9 +922,9 @@ pub struct TestReachabilityVisitor<'tcx, 'a> {
 impl<'tcx, 'a> TestReachabilityVisitor<'tcx, 'a> {
     fn access_level_diagnostic(&mut self, def_id: LocalDefId) {
         if self.tcx.has_attr(def_id.to_def_id(), sym::rustc_effective_visibility) {
+            let mut error_msg = String::new();
+            let span = self.tcx.def_span(def_id.to_def_id());
             if let Some(effective_vis) = self.access_levels.get_effective_vis(def_id) {
-                let mut error_msg = String::new();
-                let span = self.tcx.def_span(def_id.to_def_id());
                 for level in AccessLevel::all_levels() {
                     let vis_str = match effective_vis.get(level) {
                         ty::Visibility::Restricted(restricted_id) => {
@@ -943,8 +943,10 @@ impl<'tcx, 'a> TestReachabilityVisitor<'tcx, 'a> {
                     }
                     error_msg.push_str(&format!("{:?}: {}", level, vis_str));
                 }
-                self.tcx.sess.emit_err(ReportEffectiveVisibility { span, descr: error_msg });
+            } else {
+                error_msg.push_str("not in the table");
             }
+            self.tcx.sess.emit_err(ReportEffectiveVisibility { span, descr: error_msg });
         }
     }
 }

compiler/rustc_resolve/src/access_levels.rs

@@ -96,25 +96,25 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
         parent_id: LocalDefId,
         tag: AccessLevel,
     ) {
+        let module_id = self
+            .r
+            .get_nearest_non_block_module(def_id.to_def_id())
+            .nearest_parent_mod()
+            .expect_local();
+        if nominal_vis == Visibility::Restricted(module_id)
+            || self.r.visibilities[&parent_id] == Visibility::Restricted(module_id)
+        {
+            return;
+        }
         let mut access_levels = std::mem::take(&mut self.r.access_levels);
-        let module_id =
-            self.r.get_nearest_non_block_module(def_id.to_def_id()).def_id().expect_local();
-        let res = access_levels.update(
+        self.changed |= access_levels.update(
             def_id,
             nominal_vis,
             || Visibility::Restricted(module_id),
             parent_id,
             tag,
             &*self.r,
         );
-        if let Ok(changed) = res {
-            self.changed |= changed;
-        } else {
-            self.r.session.delay_span_bug(
-                self.r.opt_span(def_id.to_def_id()).unwrap(),
-                "Can't update effective visibility",
-            );
-        }
         self.r.access_levels = access_levels;
     }
 }

src/test/ui/privacy/access_levels.rs

@@ -17,12 +17,12 @@ mod outer { //~ ERROR Public: pub(crate), Exported: pub(crate), Reachable: pub(c
         }
 
         #[rustc_effective_visibility]
-        struct PrivStruct; //~ ERROR Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
+        struct PrivStruct; //~ ERROR not in the table
 
         #[rustc_effective_visibility]
         pub union PubUnion { //~ ERROR Public: pub(crate), Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
             #[rustc_effective_visibility]
-            a: u8, //~ ERROR Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
+            a: u8, //~ ERROR not in the table
             #[rustc_effective_visibility]
             pub b: u8, //~ ERROR Public: pub(crate), Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
         }
@@ -38,13 +38,13 @@ mod outer { //~ ERROR Public: pub(crate), Exported: pub(crate), Reachable: pub(c
     }
 
     #[rustc_effective_visibility]
-    macro_rules! none_macro { //~ Public: pub(crate), Exported: pub(crate), Reachable: pub(crate), ReachableFromImplTrait: pub(crate)
+    macro_rules! none_macro { //~ ERROR Public: pub(crate), Exported: pub(crate), Reachable: pub(crate), ReachableFromImplTrait: pub(crate)
         () => {};
     }
 
     #[macro_export]
     #[rustc_effective_visibility]
-    macro_rules! public_macro { //~ Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
+    macro_rules! public_macro { //~ ERROR Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
         () => {};
     }
 

src/test/ui/privacy/access_levels.stderr

@@ -22,7 +22,7 @@ error: Public: pub(crate), Exported: pub, Reachable: pub, ReachableFromImplTrait
 LL |         pub trait PubTrait {
    |         ^^^^^^^^^^^^^^^^^^
 
-error: Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
+error: not in the table
   --> $DIR/access_levels.rs:20:9
    |
 LL |         struct PrivStruct;
@@ -34,7 +34,7 @@ error: Public: pub(crate), Exported: pub, Reachable: pub, ReachableFromImplTrait
 LL |         pub union PubUnion {
    |         ^^^^^^^^^^^^^^^^^^
 
-error: Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
+error: not in the table
   --> $DIR/access_levels.rs:25:13
    |
 LL |             a: u8,