Rollup merge of #144468 - petrochenkov:resolution, r=lqd,SparrowLii

resolve: Do not create `NameResolutions` on access unless necessary

`fn resolution` now just performs the access, and `fn resolution_or_default` will insert a default entry if the entry is missing.

Author: matthiaskrgr

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -452,8 +452,10 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
                     self.r.per_ns(|this, ns| {
                         if !type_ns_only || ns == TypeNS {
                             let key = BindingKey::new(target, ns);
-                            let mut resolution = this.resolution(current_module, key).borrow_mut();
-                            resolution.single_imports.insert(import);
+                            this.resolution_or_default(current_module, key)
+                                .borrow_mut()
+                                .single_imports
+                                .insert(import);
                         }
                     });
                 }

compiler/rustc_resolve/src/check_unused.rs

@@ -509,9 +509,7 @@ impl Resolver<'_, '_> {
         let mut check_redundant_imports = FxIndexSet::default();
         for module in self.arenas.local_modules().iter() {
             for (_key, resolution) in self.resolutions(*module).borrow().iter() {
-                let resolution = resolution.borrow();
-
-                if let Some(binding) = resolution.best_binding()
+                if let Some(binding) = resolution.borrow().best_binding()
                     && let NameBindingKind::Import { import, .. } = binding.kind
                     && let ImportKind::Single { id, .. } = import.kind
                 {

compiler/rustc_resolve/src/diagnostics.rs

@@ -2659,10 +2659,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             return None;
         }
 
-        let resolutions = self.resolutions(crate_module).borrow();
         let binding_key = BindingKey::new(ident, MacroNS);
-        let resolution = resolutions.get(&binding_key)?;
-        let binding = resolution.borrow().binding()?;
+        let binding = self.resolution(crate_module, binding_key)?.binding()?;
         let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() else {
             return None;
         };

compiler/rustc_resolve/src/effective_visibilities.rs

@@ -114,9 +114,7 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
     /// including their whole reexport chains.
     fn set_bindings_effective_visibilities(&mut self, module_id: LocalDefId) {
         let module = self.r.expect_module(module_id.to_def_id());
-        let resolutions = self.r.resolutions(module);
-
-        for (_, name_resolution) in resolutions.borrow().iter() {
+        for (_, name_resolution) in self.r.resolutions(module).borrow().iter() {
             let Some(mut binding) = name_resolution.borrow().binding() else {
                 continue;
             };

compiler/rustc_resolve/src/ident.rs

@@ -848,8 +848,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         };
 
         let key = BindingKey::new(ident, ns);
-        let resolution =
-            self.resolution(module, key).try_borrow_mut().map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports.
+        // `try_borrow_mut` is required to ensure exclusive access, even if the resulting binding
+        // doesn't need to be mutable. It will fail when there is a cycle of imports, and without
+        // the exclusive access infinite recursion will crash the compiler with stack overflow.
+        let resolution = &*self
+            .resolution_or_default(module, key)
+            .try_borrow_mut()
+            .map_err(|_| (Determined, Weak::No))?;
 
         // If the primary binding is unusable, search further and return the shadowed glob
         // binding if it exists. What we really want here is having two separate scopes in

compiler/rustc_resolve/src/imports.rs

@@ -469,7 +469,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         // Ensure that `resolution` isn't borrowed when defining in the module's glob importers,
         // during which the resolution might end up getting re-defined via a glob cycle.
         let (binding, t, warn_ambiguity) = {
-            let resolution = &mut *self.resolution(module, key).borrow_mut();
+            let resolution = &mut *self.resolution_or_default(module, key).borrow_mut();
             let old_binding = resolution.binding();
 
             let t = f(self, resolution);
@@ -651,7 +651,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         for module in self.arenas.local_modules().iter() {
             for (key, resolution) in self.resolutions(*module).borrow().iter() {
                 let resolution = resolution.borrow();
-
                 let Some(binding) = resolution.best_binding() else { continue };
 
                 if let NameBindingKind::Import { import, .. } = binding.kind
@@ -1202,41 +1201,39 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             });
 
             return if all_ns_failed {
-                let resolutions = match module {
-                    ModuleOrUniformRoot::Module(module) => Some(self.resolutions(module).borrow()),
-                    _ => None,
-                };
-                let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter());
-                let names = resolutions
-                    .filter_map(|(BindingKey { ident: i, .. }, resolution)| {
-                        if i.name == ident.name {
-                            return None;
-                        } // Never suggest the same name
-                        match *resolution.borrow() {
-                            ref resolution
-                                if let Some(name_binding) = resolution.best_binding() =>
-                            {
-                                match name_binding.kind {
-                                    NameBindingKind::Import { binding, .. } => {
-                                        match binding.kind {
-                                            // Never suggest the name that has binding error
-                                            // i.e., the name that cannot be previously resolved
-                                            NameBindingKind::Res(Res::Err) => None,
-                                            _ => Some(i.name),
+                let names = match module {
+                    ModuleOrUniformRoot::Module(module) => {
+                        self.resolutions(module)
+                            .borrow()
+                            .iter()
+                            .filter_map(|(BindingKey { ident: i, .. }, resolution)| {
+                                if i.name == ident.name {
+                                    return None;
+                                } // Never suggest the same name
+
+                                let resolution = resolution.borrow();
+                                if let Some(name_binding) = resolution.best_binding() {
+                                    match name_binding.kind {
+                                        NameBindingKind::Import { binding, .. } => {
+                                            match binding.kind {
+                                                // Never suggest the name that has binding error
+                                                // i.e., the name that cannot be previously resolved
+                                                NameBindingKind::Res(Res::Err) => None,
+                                                _ => Some(i.name),
+                                            }
                                         }
+                                        _ => Some(i.name),
                                     }
-                                    _ => Some(i.name),
+                                } else if resolution.single_imports.is_empty() {
+                                    None
+                                } else {
+                                    Some(i.name)
                                 }
-                            }
-                            NameResolution { ref single_imports, .. }
-                                if single_imports.is_empty() =>
-                            {
-                                None
-                            }
-                            _ => Some(i.name),
-                        }
-                    })
-                    .collect::<Vec<Symbol>>();
+                            })
+                            .collect()
+                    }
+                    _ => Vec::new(),
+                };
 
                 let lev_suggestion =
                     find_best_match_for_name(&names, ident.name, None).map(|suggestion| {
@@ -1517,8 +1514,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 let imported_binding = self.import(binding, import);
                 let warn_ambiguity = self
                     .resolution(import.parent_scope.module, key)
-                    .borrow()
-                    .binding()
+                    .and_then(|r| r.binding())
                     .is_some_and(|binding| binding.warn_ambiguity_recursive());
                 let _ = self.try_define(
                     import.parent_scope.module,

compiler/rustc_resolve/src/late.rs

@@ -3449,8 +3449,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
         };
         ident.span.normalize_to_macros_2_0_and_adjust(module.expansion);
         let key = BindingKey::new(ident, ns);
-        let mut binding =
-            self.r.resolution(module, key).try_borrow().ok().and_then(|r| r.best_binding());
+        let mut binding = self.r.resolution(module, key).and_then(|r| r.best_binding());
         debug!(?binding);
         if binding.is_none() {
             // We could not find the trait item in the correct namespace.
@@ -3461,8 +3460,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
                 _ => ns,
             };
             let key = BindingKey::new(ident, ns);
-            binding =
-                self.r.resolution(module, key).try_borrow().ok().and_then(|r| r.best_binding());
+            binding = self.r.resolution(module, key).and_then(|r| r.best_binding());
             debug!(?binding);
         }
 

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -1461,15 +1461,17 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
             if let PathResult::Module(ModuleOrUniformRoot::Module(module)) =
                 self.resolve_path(mod_path, None, None)
             {
-                let resolutions = self.r.resolutions(module).borrow();
-                let targets: Vec<_> = resolutions
+                let targets: Vec<_> = self
+                    .r
+                    .resolutions(module)
+                    .borrow()
                     .iter()
                     .filter_map(|(key, resolution)| {
                         resolution
                             .borrow()
                             .best_binding()
                             .map(|binding| binding.res())
-                            .and_then(|res| if filter_fn(res) { Some((key, res)) } else { None })
+                            .and_then(|res| if filter_fn(res) { Some((*key, res)) } else { None })
                     })
                     .collect();
                 if let [target] = targets.as_slice() {
@@ -2300,8 +2302,9 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
             return None;
         }
 
-        let resolutions = self.r.resolutions(*module);
-        let targets = resolutions
+        let targets = self
+            .r
+            .resolutions(*module)
             .borrow()
             .iter()
             .filter_map(|(key, res)| {

compiler/rustc_resolve/src/lib.rs

@@ -21,7 +21,7 @@
 #![recursion_limit = "256"]
 // tidy-alphabetical-end
 
-use std::cell::{Cell, RefCell};
+use std::cell::{Cell, Ref, RefCell};
 use std::collections::BTreeSet;
 use std::fmt;
 use std::sync::Arc;
@@ -1905,9 +1905,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         &mut self,
         module: Module<'ra>,
         key: BindingKey,
+    ) -> Option<Ref<'ra, NameResolution<'ra>>> {
+        self.resolutions(module).borrow().get(&key).map(|resolution| resolution.borrow())
+    }
+
+    fn resolution_or_default(
+        &mut self,
+        module: Module<'ra>,
+        key: BindingKey,
     ) -> &'ra RefCell<NameResolution<'ra>> {
-        *self
-            .resolutions(module)
+        self.resolutions(module)
             .borrow_mut()
             .entry(key)
             .or_insert_with(|| self.arenas.alloc_name_resolution())