Auto merge of rust-lang#109437 - petrochenkov:effvisopt, r=davidtwco

resolve: Restore some effective visibility optimizations

Something similar was previously removed as a part of rust-lang#104602.
So we can see [bitmaps-3.1.0](https://github.com/rust-lang/rustc-perf/tree/master/collector/compile-benchmarks/bitmaps-3.1.0), [match-stress](https://github.com/rust-lang/rustc-perf/tree/master/collector/compile-benchmarks/match-stress) and [unused-warnings](https://github.com/rust-lang/rustc-perf/tree/master/collector/compile-benchmarks/unused-warnings) in regressions there, and in improvements in this PR.
After this PR all table changes should also be "locally correct" after every update.

Author: bors

compiler/rustc_resolve/src/effective_visibilities.rs

@@ -155,10 +155,6 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
         }
     }
 
-    fn cheap_private_vis(&self, parent_id: ParentId<'_>) -> Option<Visibility> {
-        matches!(parent_id, ParentId::Def(_)).then_some(self.current_private_vis)
-    }
-
     fn effective_vis_or_private(&mut self, parent_id: ParentId<'a>) -> EffectiveVisibility {
         // Private nodes are only added to the table for caching, they could be added or removed at
         // 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> {
         }
     }
 
+    /// All effective visibilities for a node are larger or equal than private visibility
+    /// for that node (see `check_invariants` in middle/privacy.rs).
+    /// So if either parent or nominal visibility is the same as private visibility, then
+    /// `min(parent_vis, nominal_vis) <= private_vis`, and the update logic is guaranteed
+    /// to not update anything and we can skip it.
+    ///
+    /// We are checking this condition only if the correct value of private visibility is
+    /// cheaply available, otherwise it does't make sense performance-wise.
+    ///
+    /// `None` is returned if the update can be skipped,
+    /// and cheap private visibility is returned otherwise.
+    fn may_update(
+        &self,
+        nominal_vis: Visibility,
+        parent_id: ParentId<'_>,
+    ) -> Option<Option<Visibility>> {
+        match parent_id {
+            ParentId::Def(def_id) => (nominal_vis != self.current_private_vis
+                && self.r.visibilities[&def_id] != self.current_private_vis)
+                .then_some(Some(self.current_private_vis)),
+            ParentId::Import(_) => Some(None),
+        }
+    }
+
     fn update_import(&mut self, binding: ImportId<'a>, parent_id: ParentId<'a>) {
         let nominal_vis = binding.vis.expect_local();
-        let private_vis = self.cheap_private_vis(parent_id);
+        let Some(cheap_private_vis) = self.may_update(nominal_vis, parent_id) else { return };
         let inherited_eff_vis = self.effective_vis_or_private(parent_id);
         let tcx = self.r.tcx;
         self.changed |= self.import_effective_visibilities.update(
             binding,
             nominal_vis,
-            || private_vis.unwrap_or_else(|| self.r.private_vis_import(binding)),
+            || cheap_private_vis.unwrap_or_else(|| self.r.private_vis_import(binding)),
             inherited_eff_vis,
             parent_id.level(),
             tcx,
         );
     }
 
     fn update_def(&mut self, def_id: LocalDefId, nominal_vis: Visibility, parent_id: ParentId<'a>) {
-        let private_vis = self.cheap_private_vis(parent_id);
+        let Some(cheap_private_vis) = self.may_update(nominal_vis, parent_id) else { return };
         let inherited_eff_vis = self.effective_vis_or_private(parent_id);
         let tcx = self.r.tcx;
         self.changed |= self.def_effective_visibilities.update(
             def_id,
             nominal_vis,
-            || private_vis.unwrap_or_else(|| self.r.private_vis_def(def_id)),
+            || cheap_private_vis.unwrap_or_else(|| self.r.private_vis_def(def_id)),
             inherited_eff_vis,
             parent_id.level(),
             tcx,

tests/ui/privacy/effective_visibilities.rs

@@ -18,13 +18,13 @@ mod outer { //~ ERROR Direct: pub(crate), Reexported: pub(crate), Reachable: pub
         }
 
         #[rustc_effective_visibility]
-        struct PrivStruct; //~ ERROR Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
-                           //~| ERROR Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
+        struct PrivStruct; //~ ERROR not in the table
+                           //~| ERROR not in the table
 
         #[rustc_effective_visibility]
         pub union PubUnion { //~ ERROR Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
             #[rustc_effective_visibility]
-            a: u8, //~ ERROR Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
+            a: u8, //~ ERROR not in the table
             #[rustc_effective_visibility]
             pub b: u8, //~ ERROR Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
         }

tests/ui/privacy/effective_visibilities.stderr

@@ -34,13 +34,13 @@ error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImpl
 LL |         pub trait PubTrait {
    |         ^^^^^^^^^^^^^^^^^^
 
-error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
+error: not in the table
   --> $DIR/effective_visibilities.rs:21:9
    |
 LL |         struct PrivStruct;
    |         ^^^^^^^^^^^^^^^^^
 
-error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
+error: not in the table
   --> $DIR/effective_visibilities.rs:21:9
    |
 LL |         struct PrivStruct;
@@ -52,7 +52,7 @@ error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImpl
 LL |         pub union PubUnion {
    |         ^^^^^^^^^^^^^^^^^^
 
-error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
+error: not in the table
   --> $DIR/effective_visibilities.rs:27:13
    |
 LL |             a: u8,