From edcbc4cb16a248766a81d6ed2e1d61ab8b60f72e Mon Sep 17 00:00:00 2001 From: b-naber Date: Fri, 18 Jul 2025 10:31:59 +0000 Subject: [PATCH 1/3] introduce Scope::NonGlobModule and Scope::GlobModule --- compiler/rustc_resolve/src/diagnostics.rs | 8 +- compiler/rustc_resolve/src/ident.rs | 420 ++++++++++++++++------ compiler/rustc_resolve/src/lib.rs | 14 +- 3 files changed, 337 insertions(+), 105 deletions(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 517e20e061966..4dcfbb43676db 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1076,7 +1076,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } } - Scope::Module(module, _) => { + Scope::NonGlobModule(module, _) | Scope::GlobModule(module, _) => { this.add_module_candidates(module, &mut suggestions, filter_fn, None); } Scope::MacroUsePrelude => { @@ -1485,9 +1485,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { &parent_scope, ident.span.ctxt(), |this, scope, _use_prelude, _ctxt| { - let Scope::Module(m, _) = scope else { - return None; + let m = match scope { + Scope::NonGlobModule(module, _) | Scope::GlobModule(module, _) => module, + _ => return None, }; + for (_, resolution) in this.resolutions(m).borrow().iter() { let Some(binding) = resolution.borrow().best_binding() else { continue; diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 092bb6fc4f989..93ffad8363ef3 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -40,6 +40,18 @@ enum Shadowing { Unrestricted, } +bitflags::bitflags! { + #[derive(Clone, Copy, Debug)] + struct Flags: u8 { + const MACRO_RULES = 1 << 0; + const NON_GLOB_MODULE = 1 << 1; + const GLOB_MODULE = 1 << 2; + const MISC_SUGGEST_CRATE = 1 << 3; + const MISC_SUGGEST_SELF = 1 << 4; + const MISC_FROM_PRELUDE = 1 << 5; + } +} + impl<'ra, 'tcx> Resolver<'ra, 'tcx> { /// A generic scope visitor. /// Visits scopes in order to resolve some identifier in them or perform other actions. @@ -112,8 +124,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { }; let module_and_extern_prelude = matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..)); let mut scope = match ns { - _ if module_and_extern_prelude => Scope::Module(module, None), - TypeNS | ValueNS => Scope::Module(module, None), + _ if module_and_extern_prelude => Scope::NonGlobModule(module, None), + TypeNS | ValueNS => Scope::NonGlobModule(module, None), MacroNS => Scope::DeriveHelpers(parent_scope.expansion), }; let mut ctxt = ctxt.normalize_to_macros_2_0(); @@ -140,7 +152,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } true } - Scope::Module(..) => true, + Scope::NonGlobModule(..) | Scope::GlobModule(..) => true, Scope::MacroUsePrelude => use_prelude || rust_2015, Scope::BuiltinAttrs => true, Scope::ExternPrelude => use_prelude || module_and_extern_prelude, @@ -177,16 +189,22 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { MacroRulesScope::Invocation(invoc_id) => { Scope::MacroRules(self.invocation_parent_scopes[&invoc_id].macro_rules) } - MacroRulesScope::Empty => Scope::Module(module, None), + MacroRulesScope::Empty => Scope::NonGlobModule(module, None), }, - Scope::Module(..) if module_and_extern_prelude => match ns { - TypeNS => { - ctxt.adjust(ExpnId::root()); - Scope::ExternPrelude + Scope::NonGlobModule(..) | Scope::GlobModule(..) if module_and_extern_prelude => { + match ns { + TypeNS => { + ctxt.adjust(ExpnId::root()); + Scope::ExternPrelude + } + ValueNS | MacroNS => break, } - ValueNS | MacroNS => break, - }, - Scope::Module(module, prev_lint_id) => { + } + Scope::NonGlobModule(module, prev_lint_id) => { + use_prelude = !module.no_implicit_prelude; + Scope::GlobModule(module, prev_lint_id) + } + Scope::GlobModule(module, prev_lint_id) => { use_prelude = !module.no_implicit_prelude; let derive_fallback_lint_id = match scope_set { ScopeSet::Late(.., lint_id) => lint_id, @@ -194,7 +212,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { }; match self.hygienic_lexical_parent(module, &mut ctxt, derive_fallback_lint_id) { Some((parent_module, lint_id)) => { - Scope::Module(parent_module, lint_id.or(prev_lint_id)) + Scope::NonGlobModule(parent_module, lint_id.or(prev_lint_id)) } None => { ctxt.adjust(ExpnId::root()); @@ -391,17 +409,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ignore_binding: Option>, ignore_import: Option>, ) -> Result, Determinacy> { - bitflags::bitflags! { - #[derive(Clone, Copy)] - struct Flags: u8 { - const MACRO_RULES = 1 << 0; - const MODULE = 1 << 1; - const MISC_SUGGEST_CRATE = 1 << 2; - const MISC_SUGGEST_SELF = 1 << 3; - const MISC_FROM_PRELUDE = 1 << 4; - } - } - assert!(force || finalize.is_none()); // `finalize` implies `force` // Make sure `self`, `super` etc produce an error when passed to here. @@ -493,7 +500,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { MacroRulesScope::Invocation(_) => Err(Determinacy::Undetermined), _ => Err(Determinacy::Determined), }, - Scope::Module(module, derive_fallback_lint_id) => { + Scope::NonGlobModule(module, derive_fallback_lint_id) => { let (adjusted_parent_scope, finalize) = if matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..)) { (parent_scope, finalize) @@ -503,8 +510,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { finalize.map(|f| Finalize { used: Used::Scope, ..f }), ) }; - let binding = this.reborrow().resolve_ident_in_module_unadjusted( - ModuleOrUniformRoot::Module(module), + + let binding = this.reborrow().resolve_ident_in_non_glob_module_unadjusted( + module, ident, ns, adjusted_parent_scope, @@ -517,6 +525,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ignore_binding, ignore_import, ); + match binding { Ok(binding) => { if let Some(lint_id) = derive_fallback_lint_id { @@ -531,14 +540,62 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { }, ); } - let misc_flags = if module == this.graph_root { - Flags::MISC_SUGGEST_CRATE - } else if module.is_normal() { - Flags::MISC_SUGGEST_SELF - } else { - Flags::empty() - }; - Ok((binding, Flags::MODULE | misc_flags)) + + let misc_flags = this.create_module_misc_flags(module); + Ok((binding, Flags::NON_GLOB_MODULE | misc_flags)) + } + Err((Determinacy::Undetermined, Weak::No)) => { + return Some(Err(Determinacy::determined(force))); + } + Err((Determinacy::Undetermined, Weak::Yes)) => { + Err(Determinacy::Undetermined) + } + Err((Determinacy::Determined, _)) => Err(Determinacy::Determined), + } + } + Scope::GlobModule(module, derive_fallback_lint_id) => { + let (adjusted_parent_scope, finalize) = + if matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..)) { + (parent_scope, finalize) + } else { + ( + &ParentScope { module, ..*parent_scope }, + finalize.map(|f| Finalize { used: Used::Scope, ..f }), + ) + }; + + let binding = this.reborrow().resolve_ident_in_glob_module_unadjusted( + module, + ident, + ns, + adjusted_parent_scope, + if matches!(scope_set, ScopeSet::Late(..)) { + Shadowing::Unrestricted + } else { + Shadowing::Restricted + }, + finalize.map(|finalize| Finalize { used: Used::Scope, ..finalize }), + ignore_binding, + ignore_import, + ); + + match binding { + Ok(binding) => { + if let Some(lint_id) = derive_fallback_lint_id { + this.get_mut().lint_buffer.buffer_lint( + PROC_MACRO_DERIVE_RESOLUTION_FALLBACK, + lint_id, + orig_ident.span, + BuiltinLintDiag::ProcMacroDeriveResolutionFallback { + span: orig_ident.span, + ns, + ident, + }, + ); + } + + let misc_flags = this.create_module_misc_flags(module); + Ok((binding, Flags::GLOB_MODULE | misc_flags)) } Err((Determinacy::Undetermined, Weak::No)) => { return Some(Err(Determinacy::determined(force))); @@ -653,7 +710,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { if let Some((innermost_binding, innermost_flags)) = innermost_result { // Found another solution, if the first one was "weak", report an error. let (res, innermost_res) = (binding.res(), innermost_binding.res()); - if res != innermost_res { + + // don't report ambiguity errors when we have a glob resolution with a + // non-glob innermost resolution. + let ignore_innermost_result = flags.contains(Flags::GLOB_MODULE) + && innermost_flags.contains(Flags::NON_GLOB_MODULE); + + if res != innermost_res && !ignore_innermost_result { let is_builtin = |res| { matches!(res, Res::NonMacroAttr(NonMacroAttrKind::Builtin(..))) }; @@ -671,13 +734,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { { Some(AmbiguityKind::DeriveHelper) } else if innermost_flags.contains(Flags::MACRO_RULES) - && flags.contains(Flags::MODULE) + && (flags.contains(Flags::NON_GLOB_MODULE) + || flags.contains(Flags::GLOB_MODULE)) && !this.disambiguate_macro_rules_vs_modularized( innermost_binding, binding, ) || flags.contains(Flags::MACRO_RULES) - && innermost_flags.contains(Flags::MODULE) + && (innermost_flags.contains(Flags::NON_GLOB_MODULE) + || flags.contains(Flags::GLOB_MODULE)) && !this.disambiguate_macro_rules_vs_modularized( binding, innermost_binding, @@ -723,7 +788,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } Err(Determinacy::Determined) => {} - Err(Determinacy::Undetermined) => determinacy = Determinacy::Undetermined, + Err(Determinacy::Undetermined) => { + if let Scope::NonGlobModule(..) = scope { + // we wait for Scope::GlobModule to determine `determinacy` + } else { + determinacy = Determinacy::Undetermined; + } + } } None @@ -742,6 +813,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { Err(Determinacy::determined(determinacy == Determinacy::Determined || force)) } + fn create_module_misc_flags(&self, module: Module<'ra>) -> Flags { + if module == self.graph_root { + Flags::MISC_SUGGEST_CRATE + } else if module.is_normal() { + Flags::MISC_SUGGEST_SELF + } else { + Flags::empty() + } + } + #[instrument(level = "debug", skip(self))] pub(crate) fn maybe_resolve_ident_in_module<'r>( self: CmResolver<'r, 'ra, 'tcx>, @@ -866,6 +947,49 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } }; + match self.reborrow().resolve_ident_in_non_glob_module_unadjusted( + module, + ident, + ns, + parent_scope, + shadowing, + finalize, + ignore_binding, + ignore_import, + ) { + Ok(binding) => return Ok(binding), + Err((_, Weak::No)) => { + return Err((Determined, Weak::No)); + } + // no non-glob binding was found, check for glob binding + Err((_, Weak::Yes)) => {} + } + + self.reborrow().resolve_ident_in_glob_module_unadjusted( + module, + ident, + ns, + parent_scope, + shadowing, + finalize, + ignore_binding, + ignore_import, + ) + } + + fn resolve_ident_in_non_glob_module_unadjusted<'r>( + mut self: CmResolver<'r, 'ra, 'tcx>, + module: Module<'ra>, + ident: Ident, + ns: Namespace, + parent_scope: &ParentScope<'ra>, + shadowing: Shadowing, + finalize: Option, + // This binding should be ignored during in-module resolution, so that we don't get + // "self-confirming" import resolutions during import validation and checking. + ignore_binding: Option>, + _ignore_import: Option>, // not used, but kept for signature consistency + ) -> Result, (Determinacy, Weak)> { let key = BindingKey::new(ident, ns); // `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 @@ -875,74 +999,134 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { .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 - // 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.non_glob_binding, resolution.glob_binding] - .into_iter() - .find_map(|binding| if binding == ignore_binding { None } else { binding }); + let check_usable = |this: CmResolver<'r, 'ra, 'tcx>, binding: NameBinding<'ra>| { + let usable = this.is_accessible_from(binding.vis, parent_scope.module); + if usable { Ok(binding) } else { Err((Determined, Weak::No)) } + }; - if let Some(finalize) = finalize { - return self.get_mut().finalize_module_binding( - ident, - binding, - if resolution.non_glob_binding.is_some() { resolution.glob_binding } else { None }, - parent_scope, - finalize, - shadowing, - ); + if let Some(binding) = resolution.non_glob_binding + && ignore_binding.map_or(true, |b| binding != b) + { + if let Some(finalize) = finalize { + return self.get_mut().finalize_non_glob_module_binding( + ident, + binding, + resolution.glob_binding, + parent_scope, + finalize, + shadowing, + ); + } else { + return check_usable(self, binding); + } } + Err((Determinacy::determined(finalize.is_some()), Weak::Yes)) + } + + fn resolve_ident_in_glob_module_unadjusted<'r>( + mut self: CmResolver<'r, 'ra, 'tcx>, + module: Module<'ra>, + ident: Ident, + ns: Namespace, + parent_scope: &ParentScope<'ra>, + shadowing: Shadowing, + finalize: Option, + // This binding should be ignored during in-module resolution, so that we don't get + // "self-confirming" import resolutions during import validation and checking. + ignore_binding: Option>, + ignore_import: Option>, + ) -> Result, (Determinacy, Weak)> { + let key = BindingKey::new(ident, ns); + let resolution = self + .resolution_or_default(module, key) + .try_borrow_mut() + .map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports. + let check_usable = |this: CmResolver<'r, 'ra, 'tcx>, binding: NameBinding<'ra>| { let usable = this.is_accessible_from(binding.vis, parent_scope.module); if usable { Ok(binding) } else { Err((Determined, Weak::No)) } }; - // Items and single imports are not shadowable, if we have one, then it's determined. - if let Some(binding) = binding - && !binding.is_glob_import() + if let Some(binding) = resolution.glob_binding + && ignore_binding.map_or(true, |b| binding != b) { - return check_usable(self, binding); - } - - // --- From now on we either have a glob resolution or no resolution. --- + if let Some(finalize) = finalize { + return self.get_mut().finalize_glob_module_binding( + ident, + binding, + parent_scope, + finalize, + shadowing, + ); + } - // Check if one of single imports can still define the name, - // if it can then our result is not determined and can be invalidated. - if self.reborrow().single_import_can_define_name( - &resolution, - binding, - ns, - ignore_import, - ignore_binding, - parent_scope, - ) { - return Err((Undetermined, Weak::No)); - } + // Check if a single import can still define the name, + // if it can then our result is not determined and can be invalidated. + if self.reborrow().single_import_can_define_name( + &resolution, + Some(binding), + ns, + ignore_import, + ignore_binding, + parent_scope, + ) { + return Err((Undetermined, Weak::No)); + } - // So we have a resolution that's from a glob import. This resolution is determined - // if it cannot be shadowed by some new item/import expanded from a macro. - // This happens either if there are no unexpanded macros, or expanded names cannot - // shadow globs (that happens in macro namespace or with restricted shadowing). - // - // Additionally, any macro in any module can plant names in the root module if it creates - // `macro_export` macros, so the root module effectively has unresolved invocations if any - // module has unresolved invocations. - // However, it causes resolution/expansion to stuck too often (#53144), so, to make - // progress, we have to ignore those potential unresolved invocations from other modules - // and prohibit access to macro-expanded `macro_export` macros instead (unless restricted - // shadowing is enabled, see `macro_expanded_macro_export_errors`). - if let Some(binding) = binding { + // So we have a resolution that's from a glob import. This resolution is determined + // if it cannot be shadowed by some new item/import expanded from a macro. + // This happens either if there are no unexpanded macros, or expanded names cannot + // shadow globs (that happens in macro namespace or with restricted shadowing). + // + // Additionally, any macro in any module can plant names in the root module if it creates + // `macro_export` macros, so the root module effectively has unresolved invocations if any + // module has unresolved invocations. + // However, it causes resolution/expansion to stuck too often (#53144), so, to make + // progress, we have to ignore those potential unresolved invocations from other modules + // and prohibit access to macro-expanded `macro_export` macros instead (unless restricted + // shadowing is enabled, see `macro_expanded_macro_export_errors`). if binding.determined() || ns == MacroNS || shadowing == Shadowing::Restricted { return check_usable(self, binding); } else { return Err((Undetermined, Weak::No)); } - } + } else if finalize.is_some() { + return Err((Determined, Weak::No)); + } else { + // Check if a single import can still define the name, + // if it can then our result is not determined and can be invalidated. + if self.reborrow().single_import_can_define_name( + &resolution, + None, + ns, + ignore_import, + ignore_binding, + parent_scope, + ) { + return Err((Undetermined, Weak::No)); + } - // --- From now on we have no resolution. --- + return Err(self.create_resolution_in_module_error( + module, + ident, + ns, + parent_scope, + ignore_binding, + ignore_import, + )); + } + } + fn create_resolution_in_module_error<'r>( + mut self: CmResolver<'r, 'ra, 'tcx>, + module: Module<'ra>, + ident: Ident, + ns: Namespace, + parent_scope: &ParentScope<'ra>, + ignore_binding: Option>, + ignore_import: Option>, + ) -> (Determinacy, Weak) { // Now we are in situation when new item/import can appear only from a glob or a macro // expansion. With restricted shadowing names from globs and macro expansions cannot // shadow names from outer scopes, so we can freely fallback from module search to search @@ -952,7 +1136,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // Check if one of unexpanded macros can still define the name, // if it can then our "no resolution" result is not determined and can be invalidated. if !module.unexpanded_invocations.borrow().is_empty() { - return Err((Undetermined, Weak::Yes)); + return (Undetermined, Weak::Yes); } // Check if one of glob imports can still define the name, @@ -967,8 +1151,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let module = match glob_import.imported_module.get() { Some(ModuleOrUniformRoot::Module(module)) => module, Some(_) => continue, - None => return Err((Undetermined, Weak::Yes)), + None => return (Undetermined, Weak::Yes), }; + let tmp_parent_scope; let (mut adjusted_parent_scope, mut ident) = (parent_scope, ident.normalize_to_macros_2_0()); @@ -999,29 +1184,25 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { { continue; } - Ok(_) | Err((Undetermined, _)) => return Err((Undetermined, Weak::Yes)), + Ok(_) | Err((Undetermined, _)) => return (Undetermined, Weak::Yes), } } // No resolution and no one else can define the name - determinate error. - Err((Determined, Weak::No)) + (Determined, Weak::No) } - fn finalize_module_binding( + fn finalize_non_glob_module_binding( &mut self, ident: Ident, - binding: Option>, - shadowed_glob: Option>, + binding: NameBinding<'ra>, + glob_binding: Option>, parent_scope: &ParentScope<'ra>, finalize: Finalize, shadowing: Shadowing, ) -> Result, (Determinacy, Weak)> { let Finalize { path_span, report_private, used, root_span, .. } = finalize; - let Some(binding) = binding else { - return Err((Determined, Weak::No)); - }; - if !self.is_accessible_from(binding.vis, parent_scope.module) { if report_private { self.privacy_errors.push(PrivacyError { @@ -1038,7 +1219,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } // Forbid expanded shadowing to avoid time travel. - if let Some(shadowed_glob) = shadowed_glob + // FIXME it should be possible to output a MoreExpandedVsOuter ambiguity error + // instead of GlobVsExpanded, but that presumably has to be done in a different location. + if let Some(shadowed_glob) = glob_binding && shadowing == Shadowing::Restricted && binding.expansion != LocalExpnId::ROOT && binding.res() != shadowed_glob.res() @@ -1066,6 +1249,43 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { return Ok(binding); } + fn finalize_glob_module_binding( + &mut self, + ident: Ident, + binding: NameBinding<'ra>, + parent_scope: &ParentScope<'ra>, + finalize: Finalize, + shadowing: Shadowing, + ) -> Result, (Determinacy, Weak)> { + let Finalize { path_span, report_private, used, root_span, .. } = finalize; + + if !self.is_accessible_from(binding.vis, parent_scope.module) { + if report_private { + self.privacy_errors.push(PrivacyError { + ident, + binding, + dedup_span: path_span, + outermost_res: None, + parent_scope: *parent_scope, + single_nested: path_span != root_span, + }); + } else { + return Err((Determined, Weak::No)); + } + } + + if shadowing == Shadowing::Unrestricted + && binding.expansion != LocalExpnId::ROOT + && let NameBindingKind::Import { import, .. } = binding.kind + && matches!(import.kind, ImportKind::MacroExport) + { + self.macro_expanded_macro_export_errors.insert((path_span, binding.span)); + } + + self.record_use(ident, binding, used); + return Ok(binding); + } + // Checks if a single import can define the `Ident` corresponding to `binding`. // This is used to check whether we can definitively accept a glob as a resolution. fn single_import_can_define_name<'r>( diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 23f44ff16583e..8a69b8f4666ce 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -122,7 +122,10 @@ enum Scope<'ra> { MacroRules(MacroRulesScopeRef<'ra>), // The node ID is for reporting the `PROC_MACRO_DERIVE_RESOLUTION_FALLBACK` // lint if it should be reported. - Module(Module<'ra>, Option), + NonGlobModule(Module<'ra>, Option), + // The node ID is for reporting the `PROC_MACRO_DERIVE_RESOLUTION_FALLBACK` + // lint if it should be reported. + GlobModule(Module<'ra>, Option), MacroUsePrelude, BuiltinAttrs, ExternPrelude, @@ -1876,10 +1879,17 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } + let mut non_glob_module_inserted = false; self.cm().visit_scopes(ScopeSet::All(TypeNS), parent_scope, ctxt, |this, scope, _, _| { match scope { - Scope::Module(module, _) => { + Scope::NonGlobModule(module, _) => { this.get_mut().traits_in_module(module, assoc_item, &mut found_traits); + non_glob_module_inserted = true; + } + Scope::GlobModule(module, _) => { + if !non_glob_module_inserted { + this.get_mut().traits_in_module(module, assoc_item, &mut found_traits); + } } Scope::StdLibPrelude => { if let Some(module) = this.prelude { From d6b1848012fea00ed451fdbdffbde401018ecfab Mon Sep 17 00:00:00 2001 From: b-naber Date: Wed, 6 Aug 2025 20:41:57 +0000 Subject: [PATCH 2/3] address review --- compiler/rustc_resolve/src/diagnostics.rs | 10 ++++++---- compiler/rustc_resolve/src/ident.rs | 16 +++++++--------- compiler/rustc_resolve/src/lib.rs | 8 ++------ 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 4dcfbb43676db..7cc0650f4ed40 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1076,9 +1076,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } } - Scope::NonGlobModule(module, _) | Scope::GlobModule(module, _) => { + Scope::NonGlobModule(module, _) => { this.add_module_candidates(module, &mut suggestions, filter_fn, None); } + Scope::GlobModule(..) => { + // already inserted in `Scope::NonGlobModule` arm + } Scope::MacroUsePrelude => { suggestions.extend(this.macro_use_prelude.iter().filter_map( |(name, binding)| { @@ -1485,9 +1488,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { &parent_scope, ident.span.ctxt(), |this, scope, _use_prelude, _ctxt| { - let m = match scope { - Scope::NonGlobModule(module, _) | Scope::GlobModule(module, _) => module, - _ => return None, + let (Scope::NonGlobModule(m, _) | Scope::GlobModule(m, _)) = scope else { + return None; }; for (_, resolution) in this.resolutions(m).borrow().iter() { diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 93ffad8363ef3..e4205976e1a1e 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -191,15 +191,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } MacroRulesScope::Empty => Scope::NonGlobModule(module, None), }, - Scope::NonGlobModule(..) | Scope::GlobModule(..) if module_and_extern_prelude => { - match ns { - TypeNS => { - ctxt.adjust(ExpnId::root()); - Scope::ExternPrelude - } - ValueNS | MacroNS => break, + Scope::GlobModule(..) if module_and_extern_prelude => match ns { + TypeNS => { + ctxt.adjust(ExpnId::root()); + Scope::ExternPrelude } - } + ValueNS | MacroNS => break, + }, Scope::NonGlobModule(module, prev_lint_id) => { use_prelude = !module.no_implicit_prelude; Scope::GlobModule(module, prev_lint_id) @@ -742,7 +740,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ) || flags.contains(Flags::MACRO_RULES) && (innermost_flags.contains(Flags::NON_GLOB_MODULE) - || flags.contains(Flags::GLOB_MODULE)) + || innermost_flags.contains(Flags::GLOB_MODULE)) && !this.disambiguate_macro_rules_vs_modularized( binding, innermost_binding, diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 8a69b8f4666ce..e5af8d9e5735b 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1879,17 +1879,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } - let mut non_glob_module_inserted = false; self.cm().visit_scopes(ScopeSet::All(TypeNS), parent_scope, ctxt, |this, scope, _, _| { match scope { Scope::NonGlobModule(module, _) => { this.get_mut().traits_in_module(module, assoc_item, &mut found_traits); - non_glob_module_inserted = true; } - Scope::GlobModule(module, _) => { - if !non_glob_module_inserted { - this.get_mut().traits_in_module(module, assoc_item, &mut found_traits); - } + Scope::GlobModule(..) => { + // already inserted in `Scope::NonGlobModule` arm } Scope::StdLibPrelude => { if let Some(module) = this.prelude { From c856e8336ba2b3861acfdbe0a6cc45c5097873f5 Mon Sep 17 00:00:00 2001 From: b-naber Date: Fri, 8 Aug 2025 09:18:22 +0000 Subject: [PATCH 3/3] add ScopeSet::Module --- compiler/rustc_resolve/src/ident.rs | 154 +++++++++++++--------------- compiler/rustc_resolve/src/lib.rs | 8 ++ 2 files changed, 82 insertions(+), 80 deletions(-) diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index e4205976e1a1e..81877526797a7 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -19,7 +19,7 @@ use crate::{ AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, BindingKey, CmResolver, Determinacy, Finalize, ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot, NameBinding, NameBindingKind, ParentScope, PathResult, PrivacyError, Res, ResolutionError, - Resolver, Scope, ScopeSet, Segment, Used, Weak, errors, + Resolver, Scope, ScopeSet, Segment, Shadowing, Used, Weak, errors, }; #[derive(Copy, Clone)] @@ -34,12 +34,6 @@ impl From for bool { } } -#[derive(Debug, PartialEq, Clone, Copy)] -enum Shadowing { - Restricted, - Unrestricted, -} - bitflags::bitflags! { #[derive(Clone, Copy, Debug)] struct Flags: u8 { @@ -113,18 +107,23 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let (ns, macro_kind) = match scope_set { ScopeSet::All(ns) | ScopeSet::ModuleAndExternPrelude(ns, _) - | ScopeSet::Late(ns, ..) => (ns, None), + | ScopeSet::Module(_, ns, _) => (ns, None), + ScopeSet::Late(ns, ..) => (ns, None), ScopeSet::Macro(macro_kind) => (MacroNS, Some(macro_kind)), }; let module = match scope_set { // Start with the specified module. - ScopeSet::Late(_, module, _) | ScopeSet::ModuleAndExternPrelude(_, module) => module, + ScopeSet::Late(_, module, _) + | ScopeSet::ModuleAndExternPrelude(_, module) + | ScopeSet::Module(module, ..) => module, // Jump out of trait or enum modules, they do not act as scopes. _ => parent_scope.module.nearest_item_scope(), }; + + let module_scope = matches!(scope_set, ScopeSet::Module(..)); let module_and_extern_prelude = matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..)); let mut scope = match ns { - _ if module_and_extern_prelude => Scope::NonGlobModule(module, None), + _ if (module_and_extern_prelude || module_scope) => Scope::NonGlobModule(module, None), TypeNS | ValueNS => Scope::NonGlobModule(module, None), MacroNS => Scope::DeriveHelpers(parent_scope.expansion), }; @@ -198,6 +197,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } ValueNS | MacroNS => break, }, + Scope::GlobModule(..) if module_scope => break, Scope::NonGlobModule(module, prev_lint_id) => { use_prelude = !module.no_implicit_prelude; Scope::GlobModule(module, prev_lint_id) @@ -417,7 +417,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let (ns, macro_kind) = match scope_set { ScopeSet::All(ns) | ScopeSet::ModuleAndExternPrelude(ns, _) - | ScopeSet::Late(ns, ..) => (ns, None), + | ScopeSet::Module(_, ns, _) => (ns, None), + ScopeSet::Late(ns, ..) => (ns, None), ScopeSet::Macro(macro_kind) => (MacroNS, Some(macro_kind)), }; @@ -500,7 +501,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { }, Scope::NonGlobModule(module, derive_fallback_lint_id) => { let (adjusted_parent_scope, finalize) = - if matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..)) { + if matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..)) + || matches!(scope_set, ScopeSet::Module(..)) + { (parent_scope, finalize) } else { ( @@ -514,10 +517,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ident, ns, adjusted_parent_scope, - if matches!(scope_set, ScopeSet::Late(..)) { - Shadowing::Unrestricted - } else { - Shadowing::Restricted + match scope_set { + ScopeSet::Late(..) => Shadowing::Unrestricted, + ScopeSet::Module(_, _, shadowing) => shadowing, + _ => Shadowing::Restricted, }, finalize, ignore_binding, @@ -539,6 +542,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ); } + // Don't visit Scope::GlobModule after successful resolution in + // Scope::NonGlobModule with ScopeSet::Module. + if matches!(scope_set, ScopeSet::Module(..)) { + return Some(Ok(binding)); + } + let misc_flags = this.create_module_misc_flags(module); Ok((binding, Flags::NON_GLOB_MODULE | misc_flags)) } @@ -548,12 +557,23 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { Err((Determinacy::Undetermined, Weak::Yes)) => { Err(Determinacy::Undetermined) } - Err((Determinacy::Determined, _)) => Err(Determinacy::Determined), + Err((Determinacy::Determined, weak)) => { + // Only go through Glob Scope with `Weak::Yes` errors in ScopeSet::Module + if matches!(scope_set, ScopeSet::Module(..)) + && matches!(weak, Weak::No) + { + return Some(Err(Determinacy::Determined)); + } + + Err(Determinacy::Determined) + } } } Scope::GlobModule(module, derive_fallback_lint_id) => { let (adjusted_parent_scope, finalize) = - if matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..)) { + if matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..)) + || matches!(scope_set, ScopeSet::Module(..)) + { (parent_scope, finalize) } else { ( @@ -567,10 +587,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ident, ns, adjusted_parent_scope, - if matches!(scope_set, ScopeSet::Late(..)) { - Shadowing::Unrestricted - } else { - Shadowing::Restricted + match scope_set { + ScopeSet::Late(..) => Shadowing::Unrestricted, + ScopeSet::Module(_, _, shadowing) => shadowing, + _ => Shadowing::Restricted, }, finalize.map(|finalize| Finalize { used: Used::Scope, ..finalize }), ignore_binding, @@ -831,7 +851,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ignore_import: Option>, ) -> Result, Determinacy> { self.resolve_ident_in_module(module, ident, ns, parent_scope, None, None, ignore_import) - .map_err(|(determinacy, _)| determinacy) } #[instrument(level = "debug", skip(self))] @@ -844,7 +863,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { finalize: Option, ignore_binding: Option>, ignore_import: Option>, - ) -> Result, (Determinacy, Weak)> { + ) -> Result, Determinacy> { let tmp_parent_scope; let mut adjusted_parent_scope = parent_scope; match module { @@ -888,12 +907,20 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // "self-confirming" import resolutions during import validation and checking. ignore_binding: Option>, ignore_import: Option>, - ) -> Result, (Determinacy, Weak)> { - let module = match module { - ModuleOrUniformRoot::Module(module) => module, + ) -> Result, Determinacy> { + match module { + ModuleOrUniformRoot::Module(module) => self.early_resolve_ident_in_lexical_scope( + ident, + ScopeSet::Module(module, ns, shadowing), + parent_scope, + finalize, + finalize.is_some(), + ignore_binding, + ignore_import, + ), ModuleOrUniformRoot::ModuleAndExternPrelude(module) => { assert_eq!(shadowing, Shadowing::Unrestricted); - let binding = self.early_resolve_ident_in_lexical_scope( + self.early_resolve_ident_in_lexical_scope( ident, ScopeSet::ModuleAndExternPrelude(ns, module), parent_scope, @@ -901,22 +928,21 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { finalize.is_some(), ignore_binding, ignore_import, - ); - return binding.map_err(|determinacy| (determinacy, Weak::No)); + ) } ModuleOrUniformRoot::ExternPrelude => { assert_eq!(shadowing, Shadowing::Unrestricted); return if ns != TypeNS { - Err((Determined, Weak::No)) + Err(Determined) } else if let Some(binding) = self.reborrow().extern_prelude_get(ident, finalize.is_some()) { Ok(binding) } else if !self.graph_root.unexpanded_invocations.borrow().is_empty() { // Macro-expanded `extern crate` items can add names to extern prelude. - Err((Undetermined, Weak::No)) + Err(Undetermined) } else { - Err((Determined, Weak::No)) + Err(Determined) }; } ModuleOrUniformRoot::CurrentScope => { @@ -932,7 +958,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } - let binding = self.early_resolve_ident_in_lexical_scope( + self.early_resolve_ident_in_lexical_scope( ident, ScopeSet::All(ns), parent_scope, @@ -940,39 +966,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { finalize.is_some(), ignore_binding, ignore_import, - ); - return binding.map_err(|determinacy| (determinacy, Weak::No)); - } - }; - - match self.reborrow().resolve_ident_in_non_glob_module_unadjusted( - module, - ident, - ns, - parent_scope, - shadowing, - finalize, - ignore_binding, - ignore_import, - ) { - Ok(binding) => return Ok(binding), - Err((_, Weak::No)) => { - return Err((Determined, Weak::No)); + ) } - // no non-glob binding was found, check for glob binding - Err((_, Weak::Yes)) => {} } - - self.reborrow().resolve_ident_in_glob_module_unadjusted( - module, - ident, - ns, - parent_scope, - shadowing, - finalize, - ignore_binding, - ignore_import, - ) } fn resolve_ident_in_non_glob_module_unadjusted<'r>( @@ -1176,13 +1172,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ); match result { - Err((Determined, _)) => continue, + Err(Determined) => continue, Ok(binding) if !self.is_accessible_from(binding.vis, glob_import.parent_scope.module) => { continue; } - Ok(_) | Err((Undetermined, _)) => return (Undetermined, Weak::Yes), + Ok(_) | Err(Undetermined) => return (Undetermined, Weak::Yes), } } @@ -1332,13 +1328,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ignore_binding, ignore_import, ) { - Err((Determined, _)) => continue, + Err(Determined) => continue, Ok(binding) if !self.is_accessible_from(binding.vis, single_import.parent_scope.module) => { continue; } - Ok(_) | Err((Undetermined, _)) => { + Ok(_) | Err(Undetermined) => { return true; } } @@ -1803,17 +1799,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } let binding = if let Some(module) = module { - self.reborrow() - .resolve_ident_in_module( - module, - ident, - ns, - parent_scope, - finalize, - ignore_binding, - ignore_import, - ) - .map_err(|(determinacy, _)| determinacy) + self.reborrow().resolve_ident_in_module( + module, + ident, + ns, + parent_scope, + finalize, + ignore_binding, + ignore_import, + ) } else if let Some(ribs) = ribs && let Some(TypeNS | ValueNS) = opt_ns { diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index e5af8d9e5735b..d14ebb07375ea 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -134,6 +134,12 @@ enum Scope<'ra> { BuiltinTypes, } +#[derive(Debug, PartialEq, Clone, Copy)] +enum Shadowing { + Restricted, + Unrestricted, +} + /// Names from different contexts may want to visit different subsets of all specific scopes /// with different restrictions when looking up the resolution. /// This enum is currently used only for early resolution (imports and macros), @@ -149,6 +155,8 @@ enum ScopeSet<'ra> { /// All scopes with the given namespace, used for partially performing late resolution. /// The node id enables lints and is used for reporting them. Late(Namespace, Module<'ra>, Option), + /// Scope::NonGlobModule and Scope::GlobModule. + Module(Module<'ra>, Namespace, Shadowing), } /// Everything you need to know about a name's location to resolve it.