resolve: Fill effective visibilities for import def ids in a separate pass

This should result in less update calls than doing it repeatedly during the fix point iteration.

2 files changed, 61 insertions, 34 deletions

diff --git a/compiler/rustc_middle/src/middle/privacy.rs b/compiler/rustc_middle/src/middle/privacy.rs
index 11fbefefcc9..3a91522d362 100644
--- a/compiler/rustc_middle/src/middle/privacy.rs
+++ b/compiler/rustc_middle/src/middle/privacy.rs
@@ -113,8 +113,30 @@ impl EffectiveVisibilities {
         })
     }
 
-    pub fn iter(&self) -> impl Iterator<Item = (&LocalDefId, &EffectiveVisibility)> {
-        self.map.iter()
+    // FIXME: Share code with `fn update`.
+    pub fn update_eff_vis(
+        &mut self,
+        def_id: LocalDefId,
+        eff_vis: &EffectiveVisibility,
+        tree: impl DefIdTree,
+    ) {
+        use std::collections::hash_map::Entry;
+        match self.map.entry(def_id) {
+            Entry::Occupied(mut occupied) => {
+                let old_eff_vis = occupied.get_mut();
+                for l in Level::all_levels() {
+                    let vis_at_level = eff_vis.at_level(l);
+                    let old_vis_at_level = old_eff_vis.at_level_mut(l);
+                    if vis_at_level != old_vis_at_level
+                        && vis_at_level.is_at_least(*old_vis_at_level, tree)
+                    {
+                        *old_vis_at_level = *vis_at_level
+                    }
+                }
+                old_eff_vis
+            }
+            Entry::Vacant(vacant) => vacant.insert(*eff_vis),
+        };
     }
 
     pub fn set_public_at_level(
@@ -185,6 +207,10 @@ impl EffectiveVisibilities {
 }
 
 impl<Id: Eq + Hash> EffectiveVisibilities<Id> {
+    pub fn iter(&self) -> impl Iterator<Item = (&Id, &EffectiveVisibility)> {
+        self.map.iter()
+    }
+
     pub fn effective_vis(&self, id: Id) -> Option<&EffectiveVisibility> {
         self.map.get(&id)
     }
diff --git a/compiler/rustc_resolve/src/effective_visibilities.rs b/compiler/rustc_resolve/src/effective_visibilities.rs
index 56dde6f8ca7..fa6d34be0cc 100644
--- a/compiler/rustc_resolve/src/effective_visibilities.rs
+++ b/compiler/rustc_resolve/src/effective_visibilities.rs
@@ -55,6 +55,38 @@ impl<'r, 'a> EffectiveVisibilitiesVisitor<'r, 'a> {
             visit::walk_crate(&mut visitor, krate);
         }
 
+        // Update visibilities for import def ids. These are not used during the
+        // `EffectiveVisibilitiesVisitor` pass, because we have more detailed binding-based
+        // information, but are used by later passes. Effective visibility of an import def id
+        // is the maximum value among visibilities of bindings corresponding to that def id.
+        for (binding, eff_vis) in visitor.import_effective_visibilities.iter() {
+            let NameBindingKind::Import { import, .. } = binding.kind else { unreachable!() };
+            if let Some(node_id) = import.id() {
+                let mut update = |node_id| {
+                    r.effective_visibilities.update_eff_vis(
+                        r.local_def_id(node_id),
+                        eff_vis,
+                        ResolverTree(&r.definitions, &r.crate_loader),
+                    )
+                };
+                update(node_id);
+                if let ImportKind::Single { additional_ids: (id1, id2), .. } = import.kind {
+                    // In theory all the single import IDs have individual visibilities and
+                    // effective visibilities, but in practice these IDs go straigth to HIR
+                    // where all their few uses assume that their (effective) visibility
+                    // applies to the whole syntactic `use` item. So they all get the same
+                    // value which is the maximum of all bindings. Maybe HIR for imports
+                    // shouldn't use three IDs at all.
+                    if id1 != ast::DUMMY_NODE_ID {
+                        update(id1);
+                    }
+                    if id2 != ast::DUMMY_NODE_ID {
+                        update(id2);
+                    }
+                }
+            }
+        }
+
         info!("resolve::effective_visibilities: {:#?}", r.effective_visibilities);
     }
 
@@ -75,41 +107,10 @@ impl<'r, 'a> EffectiveVisibilitiesVisitor<'r, 'a> {
                 // sets the rest of the `use` chain to `Level::Reexported` until
                 // we hit the actual exported item.
                 let mut parent_id = ParentId::Def(module_id);
-                while let NameBindingKind::Import { binding: nested_binding, import, .. } =
-                    binding.kind
-                {
+                while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind {
                     let binding_id = ImportId::new_unchecked(binding);
                     self.update_import(binding_id, parent_id);
 
-                    // Update visibilities for import ids. These are not used during this pass,
-                    // because we have more detailed binding-based information, but are used by
-                    // later passes. Effective visibility of an import def id is the maximum value
-                    // among visibilities of bindings corresponding to that def id.
-                    if let Some(node_id) = import.id() {
-                        let mut update = |node_id| {
-                            self.update_def(
-                                self.r.local_def_id(node_id),
-                                binding.vis.expect_local(),
-                                parent_id,
-                            )
-                        };
-                        update(node_id);
-                        if let ImportKind::Single { additional_ids: (id1, id2), .. } = import.kind {
-                            // In theory all the single import IDs have individual visibilities and
-                            // effective visibilities, but in practice these IDs go straigth to HIR
-                            // where all their few uses assume that their (effective) visibility
-                            // applies to the whole syntactic `use` item. So they all get the same
-                            // value which is the maximum of all bindings. Maybe HIR for imports
-                            // shouldn't use three IDs at all.
-                            if id1 != ast::DUMMY_NODE_ID {
-                                update(id1);
-                            }
-                            if id2 != ast::DUMMY_NODE_ID {
-                                update(id2);
-                            }
-                        }
-                    }
-
                     parent_id = ParentId::Import(binding_id);
                     binding = nested_binding;
                 }