replace binding and shadowed_glob on NameResolution with non_glob_binding and glob_binding

Author: b-naber

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.late_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().late_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().late_binding() else {
                         continue;
                     };
                     let Res::Def(DefKind::Macro(MacroKind::Derive | MacroKind::Attr), def_id) =

compiler/rustc_resolve/src/ident.rs

@@ -875,7 +875,7 @@ 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 });
 
@@ -900,7 +900,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             }
 
             // Forbid expanded shadowing to avoid time travel.
-            if let Some(shadowed_glob) = resolution.shadowed_glob
+            if let Some(shadowed_glob) = resolution.glob_binding
                 && shadowing == Shadowing::Restricted
                 && binding.expansion != LocalExpnId::ROOT
                 && binding.res() != shadowed_glob.res()

compiler/rustc_resolve/src/imports.rs

@@ -22,7 +22,7 @@ use rustc_span::edit_distance::find_best_match_for_name;
 use rustc_span::hygiene::LocalExpnId;
 use rustc_span::{Ident, Span, Symbol, kw, sym};
 use smallvec::SmallVec;
-use tracing::debug;
+use tracing::{debug, instrument};
 
 use crate::Determinacy::{self, *};
 use crate::Namespace::*;
@@ -242,21 +242,20 @@ 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 least shadowable known non-glob binding for this name, or None if there are no known bindings.
+    pub non_glob_binding: Option<NameBinding<'ra>>,
+    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| {
-            if !binding.is_glob_import() || self.single_imports.is_empty() {
-                Some(binding)
-            } else {
-                None
-            }
-        })
+        self.non_glob_binding
+            .or_else(|| if self.single_imports.is_empty() { self.glob_binding } else { None })
+    }
+
+    pub(crate) fn late_binding(&self) -> Option<NameBinding<'ra>> {
+        self.non_glob_binding.or_else(|| self.glob_binding)
     }
 }
 
@@ -338,77 +337,79 @@ 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 res == Res::Err && old_binding.res() != Res::Err {
+            if let Some(old_non_glob_binding) = resolution.non_glob_binding {
+                if res == Res::Err && old_non_glob_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) => {
-                        // 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_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(
-                                AmbiguityKind::GlobVsGlob,
-                                old_binding,
-                                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);
-                        } else if binding.is_ambiguity_recursive() {
-                            resolution.binding = Some(this.new_warn_ambiguity_binding(binding));
-                        }
+
+                if binding.is_glob_import() {
+                    let (glob_binding, nonglob_binding) = (binding, old_non_glob_binding);
+
+                    if key.ns == MacroNS
+                        && nonglob_binding.expansion != LocalExpnId::ROOT
+                        && glob_binding.res() != nonglob_binding.res()
+                    {
+                        resolution.non_glob_binding = Some(this.new_ambiguity_binding(
+                            AmbiguityKind::GlobVsExpanded,
+                            nonglob_binding,
+                            glob_binding,
+                            false,
+                        ));
                     }
-                    (old_glob @ true, false) | (old_glob @ false, true) => {
-                        let (glob_binding, nonglob_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()
-                        {
-                            resolution.binding = Some(this.new_ambiguity_binding(
-                                AmbiguityKind::GlobVsExpanded,
-                                nonglob_binding,
+
+                    if let Some(old_glob) = resolution.glob_binding {
+                        if glob_binding.res() != old_glob.res() {
+                            resolution.glob_binding = Some(this.new_ambiguity_binding(
+                                AmbiguityKind::GlobVsGlob,
+                                old_glob,
                                 glob_binding,
-                                false,
+                                warn_ambiguity,
                             ));
-                        } else {
-                            resolution.binding = Some(nonglob_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(
-                                    AmbiguityKind::GlobVsGlob,
-                                    old_shadowed_glob,
-                                    glob_binding,
-                                    false,
-                                ));
-                            } else if !old_shadowed_glob.vis.is_at_least(binding.vis, this.tcx) {
-                                resolution.shadowed_glob = Some(glob_binding);
-                            }
-                        } else {
-                            resolution.shadowed_glob = Some(glob_binding);
+                        } else if !old_glob.vis.is_at_least(glob_binding.vis, this.tcx) {
+                            resolution.glob_binding = Some(glob_binding);
                         }
+                    } else {
+                        resolution.glob_binding = Some(glob_binding);
                     }
-                    (false, false) => {
-                        return Err(old_binding);
+                } else {
+                    return Err(old_non_glob_binding);
+                }
+            } else if let Some(old_glob_binding) = resolution.glob_binding {
+                if binding.is_glob_import() {
+                    // FIXME: remove `!binding.is_ambiguity_recursive()` after delete the warning ambiguity.
+                    if !binding.is_ambiguity_recursive()
+                        && let NameBindingKind::Import { import: old_import, .. } =
+                            old_glob_binding.kind
+                        && let NameBindingKind::Import { import, .. } = 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.glob_binding = Some(binding);
+                    } else if res != old_glob_binding.res() {
+                        resolution.glob_binding = Some(this.new_ambiguity_binding(
+                            AmbiguityKind::GlobVsGlob,
+                            old_glob_binding,
+                            binding,
+                            warn_ambiguity,
+                        ));
+                    } else if !old_glob_binding.vis.is_at_least(binding.vis, this.tcx) {
+                        // We are glob-importing the same item but with greater visibility.
+                        resolution.glob_binding = Some(binding);
+                    } else if binding.is_ambiguity_recursive() {
+                        resolution.glob_binding = Some(this.new_warn_ambiguity_binding(binding));
                     }
+                } else {
+                    resolution.non_glob_binding = Some(binding);
                 }
             } else {
-                resolution.binding = Some(binding);
+                if binding.is_glob_import() {
+                    resolution.glob_binding = Some(binding);
+                } else {
+                    resolution.non_glob_binding = Some(binding);
+                }
             }
 
             Ok(())
@@ -620,15 +621,17 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         }
     }
 
+    #[instrument(skip(self), level = "debug")]
     pub(crate) fn check_hidden_glob_reexports(
         &mut self,
         exported_ambiguities: FxHashSet<NameBinding<'ra>>,
     ) {
         for module in self.arenas.local_modules().iter() {
             for (key, resolution) in self.resolutions(*module).borrow().iter() {
                 let resolution = resolution.borrow();
+                debug!(?resolution);
 
-                let Some(binding) = resolution.binding else { continue };
+                let Some(binding) = resolution.late_binding() else { continue };
 
                 if let NameBindingKind::Import { import, .. } = binding.kind
                     && let Some((amb_binding, _)) = binding.ambiguity
@@ -648,7 +651,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                     );
                 }
 
-                if let Some(glob_binding) = resolution.shadowed_glob {
+                if let Some(glob_binding) = resolution.glob_binding {
                     if binding.res() != Res::Err
                         && glob_binding.res() != Res::Err
                         && let NameBindingKind::Import { import: glob_import, .. } =
@@ -1179,7 +1182,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                             return None;
                         } // Never suggest the same name
                         match *resolution.borrow() {
-                            NameResolution { binding: Some(name_binding), .. } => {
+                            NameResolution { non_glob_binding: Some(name_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.late_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.late_binding());
             debug!(?binding);
         }
 

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -881,8 +881,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()
+                    .late_binding()
+                    .and_then(|binding| binding.res().opt_def_id())
                 else {
                     continue;
                 };
@@ -1465,15 +1467,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()
+                            .late_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));
                 }
@@ -2306,7 +2309,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().late_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,8 @@ 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 {
+            let resolution = name_resolution.borrow();
+            if let Some(binding) = resolution.non_glob_binding.or_else(|| resolution.glob_binding) {
                 f(resolver, key.ident, key.ns, binding);
             }
         }