diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 7277773334513..8bf0e71a91d85 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -18,6 +18,7 @@ use rustc_ast::visit::{self, AssocCtxt, Visitor}; use rustc_ast::{self as ast, AssocItem, AssocItemKind, MetaItemKind, StmtKind}; use rustc_ast::{Block, Fn, ForeignItem, ForeignItemKind, Impl, Item, ItemKind, NodeId}; use rustc_attr as attr; +use rustc_data_structures::intern::Interned; use rustc_data_structures::sync::Lrc; use rustc_errors::{struct_span_err, Applicability}; use rustc_expand::expand::AstFragment; @@ -378,7 +379,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { if !type_ns_only || ns == TypeNS { let key = BindingKey::new(target, ns); let mut resolution = this.resolution(current_module, key).borrow_mut(); - resolution.add_single_import(import); + resolution.single_imports.insert(Interned::new_unchecked(import)); } }); } diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index e42b2df1a5ab8..43f4a90ebfbdc 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -518,7 +518,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { ctxt: Option, ) { for (key, resolution) in self.resolutions(module).borrow().iter() { - if let Some(binding) = resolution.borrow().binding { + if let Some(binding) = resolution.borrow().available_binding() { let res = binding.res(); if filter_fn(res) && ctxt.map_or(true, |ctxt| ctxt == key.ident.span.ctxt()) { names.push(TypoSuggestion::typo_from_ident(key.ident, res)); diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index ec0a8535e7180..780151d68e911 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -870,16 +870,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let resolution = self.resolution(module, key).try_borrow_mut().map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports. - // 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 - // 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].into_iter().find_map(|binding| { - match (binding, ignore_binding) { - (Some(binding), Some(ignored)) if ptr::eq(binding, ignored) => None, - _ => binding, - } + let binding = [resolution.non_glob_binding(), resolution.glob_binding()] + .into_iter() + .find_map(|binding| match (binding, ignore_binding) { + (Some(binding), Some(ignored)) if ptr::eq(binding, ignored) => None, + _ => binding, }); if let Some(Finalize { path_span, report_private, .. }) = finalize { @@ -900,7 +895,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } // Forbid expanded shadowing to avoid time travel. - if let Some(shadowed_glob) = resolution.shadowed_glob + if let Some(shadowed_glob) = resolution.glob_binding() && restricted_shadowing && binding.expansion != LocalExpnId::ROOT && binding.res() != shadowed_glob.res() diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 47d8e5993fd82..bc4d41d5fd1ba 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -9,7 +9,7 @@ use crate::{ Resolver, Segment, }; use crate::{Finalize, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet}; -use crate::{NameBinding, NameBindingKind, PathResult}; +use crate::{NameBinding, NameBindingKind, PathResult, Res}; use rustc_ast::NodeId; use rustc_data_structures::fx::FxHashSet; @@ -34,8 +34,6 @@ use smallvec::SmallVec; use std::cell::Cell; use std::{mem, ptr}; -type Res = def::Res; - /// Contains data for specific kinds of imports. #[derive(Clone)] pub(crate) enum ImportKind<'a> { @@ -212,25 +210,32 @@ pub(crate) struct NameResolution<'a> { /// 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: FxHashSet>>, - /// The least shadowable known binding for this name, or None if there are no known bindings. - pub binding: Option<&'a NameBinding<'a>>, - pub shadowed_glob: Option<&'a NameBinding<'a>>, + non_glob_binding: Option<&'a NameBinding<'a>>, + glob_binding: Option<&'a NameBinding<'a>>, } impl<'a> NameResolution<'a> { - /// Returns the binding for the name if it is known or None if it not known. pub(crate) fn binding(&self) -> Option<&'a NameBinding<'a>> { - 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 non_glob_binding(&self) -> Option<&'a NameBinding<'a>> { + self.non_glob_binding.and_then(|binding| { + assert!(!binding.is_glob_import()); + Some(binding) + }) + } + + pub(crate) fn glob_binding(&self) -> Option<&'a NameBinding<'a>> { + self.glob_binding.and_then(|binding| { + assert!(binding.is_glob_import()); + Some(binding) }) } - pub(crate) fn add_single_import(&mut self, import: &'a Import<'a>) { - self.single_imports.insert(Interned::new_unchecked(import)); + pub(crate) fn available_binding(&self) -> Option<&'a NameBinding<'a>> { + self.non_glob_binding().or(self.glob_binding()) } } @@ -305,65 +310,41 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { self.check_reserved_macro_name(key.ident, res); self.set_binding_parent_module(binding, module); self.update_resolution(module, key, |this, resolution| { - if let Some(old_binding) = resolution.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) => { - if res != old_binding.res() { - resolution.binding = Some(this.ambiguity( - AmbiguityKind::GlobVsGlob, - old_binding, - binding, - )); - } 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); - } - } - (old_glob @ true, false) | (old_glob @ false, true) => { - let (glob_binding, nonglob_binding) = - if old_glob { (old_binding, binding) } else { (binding, old_binding) }; - if glob_binding.res() != nonglob_binding.res() - && key.ns == MacroNS - && nonglob_binding.expansion != LocalExpnId::ROOT - { - resolution.binding = Some(this.ambiguity( - AmbiguityKind::GlobVsExpanded, - nonglob_binding, - glob_binding, - )); - } else { - resolution.binding = Some(nonglob_binding); - } - - if let Some(old_binding) = resolution.shadowed_glob { - assert!(old_binding.is_glob_import()); - if glob_binding.res() != old_binding.res() { - resolution.shadowed_glob = Some(this.ambiguity( - AmbiguityKind::GlobVsGlob, - old_binding, - glob_binding, - )); - } else if !old_binding.vis.is_at_least(binding.vis, this.tcx) { - resolution.shadowed_glob = Some(glob_binding); - } - } else { - resolution.shadowed_glob = Some(glob_binding); - } - } - (false, false) => { - return Err(old_binding); - } + if let Some(old_binding) = resolution.available_binding() && res == Res::Err && old_binding.res() != Res::Err { + // Do not override real bindings with `Res::Err`s from error recovery. + Ok(()) + } else if binding.is_glob_import() { + if let Some(old_binding) = resolution.glob_binding() { + if binding.res() != old_binding.res() { + resolution.glob_binding = Some(this.ambiguity( + AmbiguityKind::GlobVsGlob, + old_binding, + binding, + )); + } else if !old_binding.vis.is_at_least(binding.vis, this.tcx) { + resolution.glob_binding = Some(binding); } } else { - resolution.binding = Some(binding); + resolution.glob_binding = Some(binding); } + if let Some(old_binding) = resolution.non_glob_binding() { + if binding.res() != old_binding.res() && key.ns == MacroNS && old_binding.expansion != LocalExpnId::ROOT { + resolution.non_glob_binding = Some(this.ambiguity( + AmbiguityKind::GlobVsExpanded, + old_binding, + binding, + )); + } + } Ok(()) - }) + } else if let Some(old_binding) = resolution.non_glob_binding() { + Err(old_binding) + } else { + resolution.non_glob_binding = Some(binding); + Ok(()) + } + }) } fn ambiguity( @@ -549,11 +530,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { for (key, resolution) in self.resolutions(module).borrow().iter() { let resolution = resolution.borrow(); - if let Some(binding) = resolution.binding { - if let NameBindingKind::Import { import, .. } = binding.kind - && let Some((amb_binding, _)) = binding.ambiguity - && binding.res() != Res::Err - && exported_ambiguities.contains(&Interned::new_unchecked(binding)) + if let Some(glob_binding) = resolution.glob_binding() { + if let NameBindingKind::Import { import, .. } = glob_binding.kind + && let Some((amb_binding, _)) = glob_binding.ambiguity + && glob_binding.res() != Res::Err + && exported_ambiguities.contains(&Interned::new_unchecked(glob_binding)) { self.lint_buffer.buffer_lint_with_diagnostic( AMBIGUOUS_GLOB_REEXPORTS, @@ -569,7 +550,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { ); } - if let Some(glob_binding) = resolution.shadowed_glob { + if let Some(binding) = resolution.non_glob_binding() { let binding_id = match binding.kind { NameBindingKind::Res(res) => { Some(self.def_id_to_node_id[res.def_id().expect_local()]) @@ -769,9 +750,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { .emit(); } let key = BindingKey::new(target, ns); - this.update_resolution(parent, key, |_, resolution| { - resolution.single_imports.remove(&Interned::new_unchecked(import)); - }); + let mut resolution = this.resolution(parent, key).borrow_mut(); + resolution.single_imports.remove(&Interned::new_unchecked(import)); } } } @@ -1025,29 +1005,25 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter()); let names = resolutions .filter_map(|(BindingKey { ident: i, .. }, resolution)| { + let resolution = resolution.borrow(); if i.name == ident.name { - return None; - } // Never suggest the same name - match *resolution.borrow() { - NameResolution { binding: Some(name_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), - } + None + } else if let Some(name_binding) = resolution.available_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), } - NameResolution { ref single_imports, .. } - if single_imports.is_empty() => - { - None - } - _ => Some(i.name), + } else if resolution.single_imports.is_empty() { + None + } else { + Some(i.name) } }) .collect::>(); diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index ddd75ea3b33e0..f5b8f5bb1a684 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -2969,7 +2969,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { let Some((module, _)) = &self.current_trait_ref else { return; }; 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.available_binding()); debug!(?binding); if binding.is_none() { // We could not find the trait item in the correct namespace. @@ -2980,7 +2981,12 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, '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.available_binding()); debug!(?binding); } let Some(binding) = binding else { diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 7284b33f09d8e..2af579bd7cc58 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1012,15 +1012,16 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, '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() + .available_binding() + .map(|binding| binding.res()) + .and_then(|res| if filter_fn(res) { Some((key, res)) } else { None }) + }) + .collect(); if targets.len() == 1 { let target = targets[0]; return Some(TypoSuggestion::single_item_from_ident(target.0.ident, target.1)); @@ -1543,7 +1544,9 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { let targets = resolutions .borrow() .iter() - .filter_map(|(key, res)| res.borrow().binding.map(|binding| (key, binding.res()))) + .filter_map(|(key, res)| { + res.borrow().available_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, diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 82b333fee28b8..9f18b9900965a 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -565,7 +565,7 @@ impl<'a> ModuleData<'a> { F: FnMut(&mut R, Ident, Namespace, &'a NameBinding<'a>), { 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().available_binding() { f(resolver, key.ident, key.ns, binding); } } diff --git a/tests/ui/resolve/non-glob-first.rs b/tests/ui/resolve/non-glob-first.rs new file mode 100644 index 0000000000000..e9c8bab0c06e9 --- /dev/null +++ b/tests/ui/resolve/non-glob-first.rs @@ -0,0 +1,21 @@ +// check-pass + +mod a { + pub trait P {} +} +pub use a::*; + +mod b { + #[derive(Clone)] + pub enum P { + A + } +} +pub use b::P; + +mod c { + use crate::*; + pub struct _S(Vec

); +} + +fn main() {}