resolve: Try removing questionable optimizations in eff vis computation

Author: petrochenkov

compiler/rustc_resolve/src/effective_visibilities.rs

@@ -38,23 +38,18 @@ pub(crate) struct EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
 }
 
 impl Resolver<'_, '_> {
-    fn nearest_normal_mod(&self, def_id: LocalDefId) -> LocalDefId {
-        self.get_nearest_non_block_module(def_id.to_def_id()).nearest_parent_mod().expect_local()
-    }
-
-    fn private_vis_import(&self, decl: Decl<'_>) -> Visibility {
-        let DeclKind::Import { import, .. } = decl.kind else { unreachable!() };
+    fn private_vis_decl(&self, decl: Decl<'_>) -> Visibility {
         Visibility::Restricted(
-            import
-                .id()
-                .map(|id| self.nearest_normal_mod(self.local_def_id(id)))
-                .unwrap_or(CRATE_DEF_ID),
+            decl.parent_module.map_or(CRATE_DEF_ID, |m| m.nearest_parent_mod().expect_local()),
         )
     }
 
     fn private_vis_def(&self, def_id: LocalDefId) -> Visibility {
-        // For mod items `nearest_normal_mod` returns its argument, but we actually need its parent.
-        let normal_mod_id = self.nearest_normal_mod(def_id);
+        // For mod items `normal_mod_id` will be equal to `def_id`, but we actually need its parent.
+        let normal_mod_id = self
+            .get_nearest_non_block_module(def_id.to_def_id())
+            .nearest_parent_mod()
+            .expect_local();
         if normal_mod_id == def_id {
             Visibility::Restricted(self.tcx.local_parent(def_id))
         } else {
@@ -120,15 +115,23 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
             // Set the given effective visibility level to `Level::Direct` and
             // sets the rest of the `use` chain to `Level::Reexported` until
             // we hit the actual exported item.
+            let orig_private_vis = self.current_private_vis;
             let mut parent_id = ParentId::Def(module_id);
             while let DeclKind::Import { source_decl, .. } = decl.kind {
+                if matches!(parent_id, ParentId::Import(_)) {
+                    self.current_private_vis = self.r.private_vis_decl(decl);
+                }
                 self.update_import(decl, parent_id);
                 parent_id = ParentId::Import(decl);
                 decl = source_decl;
             }
             if let Some(def_id) = decl.res().opt_def_id().and_then(|id| id.as_local()) {
+                if matches!(parent_id, ParentId::Import(_)) {
+                    self.current_private_vis = self.r.private_vis_decl(decl);
+                }
                 self.update_def(def_id, decl.vis().expect_local(), parent_id);
             }
+            self.current_private_vis = orig_private_vis;
         }
     }
 
@@ -141,7 +144,7 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
                 .effective_vis_or_private(def_id, || self.r.private_vis_def(def_id)),
             ParentId::Import(binding) => self
                 .import_effective_visibilities
-                .effective_vis_or_private(binding, || self.r.private_vis_import(binding)),
+                .effective_vis_or_private(binding, || self.r.private_vis_decl(binding)),
         }
     }
 
@@ -156,28 +159,30 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
     ///
     /// `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>> {
+    fn may_update(&self, nominal_vis: Visibility, parent_id: ParentId<'_>) -> bool {
         match parent_id {
-            ParentId::Def(def_id) => (nominal_vis != self.current_private_vis
-                && self.r.tcx.local_visibility(def_id) != self.current_private_vis)
-                .then_some(Some(self.current_private_vis)),
-            ParentId::Import(_) => Some(None),
+            ParentId::Def(def_id) => {
+                nominal_vis != self.current_private_vis
+                    && self.r.tcx.local_visibility(def_id) != self.current_private_vis
+            }
+            ParentId::Import(decl) => {
+                nominal_vis != self.current_private_vis
+                    && decl.vis().expect_local() != self.current_private_vis
+            }
         }
     }
 
     fn update_import(&mut self, decl: Decl<'ra>, parent_id: ParentId<'ra>) {
         let nominal_vis = decl.vis().expect_local();
-        let Some(cheap_private_vis) = self.may_update(nominal_vis, parent_id) else { return };
+        if !self.may_update(nominal_vis, parent_id) {
+            return;
+        };
         let inherited_eff_vis = self.effective_vis_or_private(parent_id);
         let tcx = self.r.tcx;
         self.changed |= self.import_effective_visibilities.update(
             decl,
             Some(nominal_vis),
-            || cheap_private_vis.unwrap_or_else(|| self.r.private_vis_import(decl)),
+            || self.current_private_vis,
             inherited_eff_vis,
             parent_id.level(),
             tcx,
@@ -194,13 +199,15 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
         nominal_vis: Visibility,
         parent_id: ParentId<'ra>,
     ) {
-        let Some(cheap_private_vis) = self.may_update(nominal_vis, parent_id) else { return };
+        if !self.may_update(nominal_vis, parent_id) {
+            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,
             Some(nominal_vis),
-            || cheap_private_vis.unwrap_or_else(|| self.r.private_vis_def(def_id)),
+            || self.current_private_vis,
             inherited_eff_vis,
             parent_id.level(),
             tcx,

compiler/rustc_resolve/src/lib.rs

@@ -1565,7 +1565,7 @@ impl<'ra> ResolverArenas<'ra> {
                 vis,
                 span,
                 LocalExpnId::ROOT,
-                None,
+                parent,
             )),
             ModuleKind::Block => None,
         };