Rollup merge of rust-lang#143734 - LorrensP-2158466:refactor-resolve-resolution-bindings, r=petrochenkov

Refactor resolve resolution bindings

This pr does the work asked in rust-lang#142547 (comment). This part:

> move the `(non)_glob_binding` change

r? ``@petrochenkov``

Author: matthiaskrgr

compiler/rustc_resolve/src/check_unused.rs

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

compiler/rustc_resolve/src/diagnostics.rs

@@ -1440,7 +1440,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         |(key, name_resolution)| {
                             if key.ns == TypeNS
                                 && key.ident == ident
-                                && let Some(binding) = name_resolution.borrow().binding
+                                && let Some(binding) = name_resolution.borrow().best_binding()
                             {
                                 match binding.res() {
                                     // No disambiguation needed if the identically named item we
@@ -1494,7 +1494,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                     return None;
                 };
                 for (_, resolution) in this.resolutions(m).borrow().iter() {
-                    let Some(binding) = resolution.borrow().binding else {
+                    let Some(binding) = resolution.borrow().best_binding() else {
                         continue;
                     };
                     let Res::Def(DefKind::Macro(MacroKind::Derive | MacroKind::Attr), def_id) =

compiler/rustc_resolve/src/ident.rs

@@ -875,15 +875,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         // binding if it exists. What we really want here is having two separate scopes in
         // a module - one for non-globs and one for globs, but until that's done use this
         // hack to avoid inconsistent resolution ICEs during import validation.
-        let binding = [resolution.binding, resolution.shadowed_glob]
+        let binding = [resolution.non_glob_binding, resolution.glob_binding]
             .into_iter()
             .find_map(|binding| if binding == ignore_binding { None } else { binding });
 
         if let Some(finalize) = finalize {
             return self.finalize_module_binding(
                 ident,
                 binding,
-                resolution.shadowed_glob,
+                if resolution.non_glob_binding.is_some() { resolution.glob_binding } else { None },
                 parent_scope,
                 finalize,
                 shadowing,

compiler/rustc_resolve/src/imports.rs

@@ -244,22 +244,27 @@ pub(crate) struct NameResolution<'ra> {
     /// Single imports that may define the name in the namespace.
     /// Imports are arena-allocated, so it's ok to use pointers as keys.
     pub single_imports: FxIndexSet<Import<'ra>>,
-    /// The least shadowable known binding for this name, or None if there are no known bindings.
-    pub binding: Option<NameBinding<'ra>>,
-    pub shadowed_glob: Option<NameBinding<'ra>>,
+    /// The non-glob binding for this name, if it is known to exist.
+    pub non_glob_binding: Option<NameBinding<'ra>>,
+    /// The glob binding for this name, if it is known to exist.
+    pub glob_binding: Option<NameBinding<'ra>>,
 }
 
 impl<'ra> NameResolution<'ra> {
     /// Returns the binding for the name if it is known or None if it not known.
     pub(crate) fn binding(&self) -> Option<NameBinding<'ra>> {
-        self.binding.and_then(|binding| {
+        self.best_binding().and_then(|binding| {
             if !binding.is_glob_import() || self.single_imports.is_empty() {
                 Some(binding)
             } else {
                 None
             }
         })
     }
+
+    pub(crate) fn best_binding(&self) -> Option<NameBinding<'ra>> {
+        self.non_glob_binding.or(self.glob_binding)
+    }
 }
 
 /// An error that may be transformed into a diagnostic later. Used to combine multiple unresolved
@@ -340,77 +345,83 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         self.check_reserved_macro_name(key.ident, res);
         self.set_binding_parent_module(binding, module);
         self.update_resolution(module, key, warn_ambiguity, |this, resolution| {
-            if let Some(old_binding) = resolution.binding {
+            if let Some(old_binding) = resolution.best_binding() {
                 if res == Res::Err && old_binding.res() != Res::Err {
                     // Do not override real bindings with `Res::Err`s from error recovery.
                     return Ok(());
                 }
                 match (old_binding.is_glob_import(), binding.is_glob_import()) {
                     (true, true) => {
+                        let (glob_binding, old_glob_binding) = (binding, old_binding);
                         // FIXME: remove `!binding.is_ambiguity_recursive()` after delete the warning ambiguity.
                         if !binding.is_ambiguity_recursive()
                             && let NameBindingKind::Import { import: old_import, .. } =
-                                old_binding.kind
-                            && let NameBindingKind::Import { import, .. } = binding.kind
+                                old_glob_binding.kind
+                            && let NameBindingKind::Import { import, .. } = glob_binding.kind
                             && old_import == import
                         {
-                            // We should replace the `old_binding` with `binding` regardless
-                            // of whether they has same resolution or not when they are
-                            // imported from the same glob-import statement.
-                            resolution.binding = Some(binding);
-                        } else if res != old_binding.res() {
-                            resolution.binding = Some(this.new_ambiguity_binding(
+                            // When imported from the same glob-import statement, we should replace
+                            // `old_glob_binding` with `glob_binding`, regardless of whether
+                            // they have the same resolution or not.
+                            resolution.glob_binding = Some(glob_binding);
+                        } else if res != old_glob_binding.res() {
+                            resolution.glob_binding = Some(this.new_ambiguity_binding(
                                 AmbiguityKind::GlobVsGlob,
-                                old_binding,
-                                binding,
+                                old_glob_binding,
+                                glob_binding,
                                 warn_ambiguity,
                             ));
                         } else if !old_binding.vis.is_at_least(binding.vis, this.tcx) {
                             // We are glob-importing the same item but with greater visibility.
-                            resolution.binding = Some(binding);
+                            resolution.glob_binding = Some(glob_binding);
                         } else if binding.is_ambiguity_recursive() {
-                            resolution.binding = Some(this.new_warn_ambiguity_binding(binding));
+                            resolution.glob_binding =
+                                Some(this.new_warn_ambiguity_binding(glob_binding));
                         }
                     }
                     (old_glob @ true, false) | (old_glob @ false, true) => {
-                        let (glob_binding, nonglob_binding) =
+                        let (glob_binding, non_glob_binding) =
                             if old_glob { (old_binding, binding) } else { (binding, old_binding) };
                         if key.ns == MacroNS
-                            && nonglob_binding.expansion != LocalExpnId::ROOT
-                            && glob_binding.res() != nonglob_binding.res()
+                            && non_glob_binding.expansion != LocalExpnId::ROOT
+                            && glob_binding.res() != non_glob_binding.res()
                         {
-                            resolution.binding = Some(this.new_ambiguity_binding(
+                            resolution.non_glob_binding = Some(this.new_ambiguity_binding(
                                 AmbiguityKind::GlobVsExpanded,
-                                nonglob_binding,
+                                non_glob_binding,
                                 glob_binding,
                                 false,
                             ));
                         } else {
-                            resolution.binding = Some(nonglob_binding);
+                            resolution.non_glob_binding = Some(non_glob_binding);
                         }
 
-                        if let Some(old_shadowed_glob) = resolution.shadowed_glob {
-                            assert!(old_shadowed_glob.is_glob_import());
-                            if glob_binding.res() != old_shadowed_glob.res() {
-                                resolution.shadowed_glob = Some(this.new_ambiguity_binding(
+                        if let Some(old_glob_binding) = resolution.glob_binding {
+                            assert!(old_glob_binding.is_glob_import());
+                            if glob_binding.res() != old_glob_binding.res() {
+                                resolution.glob_binding = Some(this.new_ambiguity_binding(
                                     AmbiguityKind::GlobVsGlob,
-                                    old_shadowed_glob,
+                                    old_glob_binding,
                                     glob_binding,
                                     false,
                                 ));
-                            } else if !old_shadowed_glob.vis.is_at_least(binding.vis, this.tcx) {
-                                resolution.shadowed_glob = Some(glob_binding);
+                            } else if !old_glob_binding.vis.is_at_least(binding.vis, this.tcx) {
+                                resolution.glob_binding = Some(glob_binding);
                             }
                         } else {
-                            resolution.shadowed_glob = Some(glob_binding);
+                            resolution.glob_binding = Some(glob_binding);
                         }
                     }
                     (false, false) => {
                         return Err(old_binding);
                     }
                 }
             } else {
-                resolution.binding = Some(binding);
+                if binding.is_glob_import() {
+                    resolution.glob_binding = Some(binding);
+                } else {
+                    resolution.non_glob_binding = Some(binding);
+                }
             }
 
             Ok(())
@@ -631,7 +642,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             for (key, resolution) in self.resolutions(*module).borrow().iter() {
                 let resolution = resolution.borrow();
 
-                let Some(binding) = resolution.binding else { continue };
+                let Some(binding) = resolution.best_binding() else { continue };
 
                 if let NameBindingKind::Import { import, .. } = binding.kind
                     && let Some((amb_binding, _)) = binding.ambiguity
@@ -651,7 +662,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                     );
                 }
 
-                if let Some(glob_binding) = resolution.shadowed_glob {
+                if let Some(glob_binding) = resolution.glob_binding
+                    && resolution.non_glob_binding.is_some()
+                {
                     if binding.res() != Res::Err
                         && glob_binding.res() != Res::Err
                         && let NameBindingKind::Import { import: glob_import, .. } =
@@ -1172,7 +1185,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                             return None;
                         } // Never suggest the same name
                         match *resolution.borrow() {
-                            NameResolution { binding: Some(name_binding), .. } => {
+                            ref resolution
+                                if let Some(name_binding) = resolution.best_binding() =>
+                            {
                                 match name_binding.kind {
                                     NameBindingKind::Import { binding, .. } => {
                                         match binding.kind {

compiler/rustc_resolve/src/late.rs

@@ -3437,7 +3437,8 @@ 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.binding);
+        let mut binding =
+            self.r.resolution(module, key).try_borrow().ok().and_then(|r| r.best_binding());
         debug!(?binding);
         if binding.is_none() {
             // We could not find the trait item in the correct namespace.
@@ -3448,7 +3449,8 @@ 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.binding);
+            binding =
+                self.r.resolution(module, key).try_borrow().ok().and_then(|r| r.best_binding());
             debug!(?binding);
         }
 

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -880,8 +880,10 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
     fn lookup_doc_alias_name(&mut self, path: &[Segment], ns: Namespace) -> Option<(DefId, Ident)> {
         let find_doc_alias_name = |r: &mut Resolver<'ra, '_>, m: Module<'ra>, item_name: Symbol| {
             for resolution in r.resolutions(m).borrow().values() {
-                let Some(did) =
-                    resolution.borrow().binding.and_then(|binding| binding.res().opt_def_id())
+                let Some(did) = resolution
+                    .borrow()
+                    .best_binding()
+                    .and_then(|binding| binding.res().opt_def_id())
                 else {
                     continue;
                 };
@@ -1464,15 +1466,16 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
                 self.resolve_path(mod_path, None, None)
             {
                 let resolutions = self.r.resolutions(module).borrow();
-                let targets: Vec<_> =
-                    resolutions
-                        .iter()
-                        .filter_map(|(key, resolution)| {
-                            resolution.borrow().binding.map(|binding| binding.res()).and_then(
-                                |res| if filter_fn(res) { Some((key, res)) } else { None },
-                            )
-                        })
-                        .collect();
+                let targets: Vec<_> = resolutions
+                    .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 })
+                    })
+                    .collect();
                 if let [target] = targets.as_slice() {
                     return Some(TypoSuggestion::single_item_from_ident(target.0.ident, target.1));
                 }
@@ -2305,7 +2308,9 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
         let targets = resolutions
             .borrow()
             .iter()
-            .filter_map(|(key, res)| res.borrow().binding.map(|binding| (key, binding.res())))
+            .filter_map(|(key, res)| {
+                res.borrow().best_binding().map(|binding| (key, binding.res()))
+            })
             .filter(|(_, res)| match (kind, res) {
                 (AssocItemKind::Const(..), Res::Def(DefKind::AssocConst, _)) => true,
                 (AssocItemKind::Fn(_), Res::Def(DefKind::AssocFn, _)) => true,

compiler/rustc_resolve/src/lib.rs

@@ -641,7 +641,7 @@ impl<'ra> Module<'ra> {
         F: FnMut(&mut R, Ident, Namespace, NameBinding<'ra>),
     {
         for (key, name_resolution) in resolver.as_mut().resolutions(self).borrow().iter() {
-            if let Some(binding) = name_resolution.borrow().binding {
+            if let Some(binding) = name_resolution.borrow().best_binding() {
                 f(resolver, key.ident, key.ns, binding);
             }
         }