rust-lang/rust
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Auto merge of #144272 - petrochenkov:disambunder2, r=oli-obk

Browse Source 
resolve: Make disambiguators for underscore bindings module-local (take 2)

The difference with https://github.com/rust-lang/rust/pull/144013 can be seen in the second commit.

Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`.
This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it.
And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once https://github.com/rust-lang/rust/pull/143884 lands.

Unblocks https://github.com/rust-lang/rust/pull/143884.
This commit is contained in:
bors
2025-07-24 09:50:20 +00:00
parent 3c30dbbe31 4b557912af
commit fc5af18133
4 changed files with 62 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
compiler/rustc_resolve/src/build_reduced_graph.rs
View File

@@ -53,8 +53,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
ns: Namespace,
binding: NameBinding<'ra>,
) {
let key = self.new_disambiguated_key(ident, ns);
if let Err(old_binding) = self.try_define(parent, key, binding, false) {
if let Err(old_binding) = self.try_define(parent, ident, ns, binding, false) {
self.report_conflict(parent, ident, ns, old_binding, binding);
}
}
@@ -446,9 +445,10 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
self.r.indeterminate_imports.push(import);
match import.kind {
// Don't add unresolved underscore imports to modules
ImportKind::Single { target: Ident { name: kw::Underscore, .. }, .. } => {}
ImportKind::Single { target, type_ns_only, .. } => {
// Don't add underscore imports to `single_imports`
// because they cannot define any usable names.
if target.name != kw::Underscore {
self.r.per_ns(|this, ns| {
if !type_ns_only || ns == TypeNS {
let key = BindingKey::new(target, ns);
@@ -457,6 +457,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
}
});
}
}
// We don't add prelude imports to the globs since they only affect lexical scopes,
// which are not relevant to import resolution.
ImportKind::Glob { is_prelude: true, .. } => {}
@@ -1401,9 +1402,12 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
let parent = self.parent_scope.module;
let expansion = self.parent_scope.expansion;
self.r.define(parent, ident, ns, self.res(def_id), vis, item.span, expansion);
} else if !matches!(&item.kind, AssocItemKind::Delegation(deleg) if deleg.from_glob) {
} else if !matches!(&item.kind, AssocItemKind::Delegation(deleg) if deleg.from_glob)
&& ident.name != kw::Underscore
{
// Don't add underscore names, they cannot be looked up anyway.
let impl_def_id = self.r.tcx.local_parent(local_def_id);
let key = BindingKey::new(ident.normalize_to_macros_2_0(), ns);
let key = BindingKey::new(ident, ns);
self.r.impl_binding_keys.entry(impl_def_id).or_default().insert(key);
}

33
compiler/rustc_resolve/src/imports.rs
View File

@@ -25,7 +25,7 @@ use rustc_span::{Ident, Span, Symbol, kw, sym};
use smallvec::SmallVec;
use tracing::debug;
use crate::Namespace::*;
use crate::Namespace::{self, *};
use crate::diagnostics::{DiagMode, Suggestion, import_candidates};
use crate::errors::{
CannotBeReexportedCratePublic, CannotBeReexportedCratePublicNS, CannotBeReexportedPrivate,
@@ -338,13 +338,21 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
pub(crate) fn try_define(
&mut self,
module: Module<'ra>,
key: BindingKey,
ident: Ident,
ns: Namespace,
binding: NameBinding<'ra>,
warn_ambiguity: bool,
) -> Result<(), NameBinding<'ra>> {
let res = binding.res();
self.check_reserved_macro_name(key.ident, res);
self.check_reserved_macro_name(ident, res);
self.set_binding_parent_module(binding, module);
// Even if underscore names cannot be looked up, we still need to add them to modules,
// because they can be fetched by glob imports from those modules, and bring traits
// into scope both directly and through glob imports.
let key = BindingKey::new_disambiguated(ident, ns, || {
module.underscore_disambiguator.update(|d| d + 1);
module.underscore_disambiguator.get()
});
self.update_resolution(module, key, warn_ambiguity, |this, resolution| {
if let Some(old_binding) = resolution.best_binding() {
if res == Res::Err && old_binding.res() != Res::Err {
@@ -383,7 +391,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
(old_glob @ true, false) | (old_glob @ false, true) => {
let (glob_binding, non_glob_binding) =
if old_glob { (old_binding, binding) } else { (binding, old_binding) };
if key.ns == MacroNS
if ns == MacroNS
&& non_glob_binding.expansion != LocalExpnId::ROOT
&& glob_binding.res() != non_glob_binding.res()
{
@@ -489,10 +497,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
};
if self.is_accessible_from(binding.vis, scope) {
let imported_binding = self.import(binding, *import);
let key = BindingKey { ident, ..key };
let _ = self.try_define(
import.parent_scope.module,
key,
ident,
key.ns,
imported_binding,
warn_ambiguity,
);
@@ -514,11 +522,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
let dummy_binding = self.dummy_binding;
let dummy_binding = self.import(dummy_binding, import);
self.per_ns(|this, ns| {
let module = import.parent_scope.module;
let _ = this.try_define(module, target, ns, dummy_binding, false);
// Don't remove underscores from `single_imports`, they were never added.
if target.name != kw::Underscore {
let key = BindingKey::new(target, ns);
let _ = this.try_define(import.parent_scope.module, key, dummy_binding, false);
this.update_resolution(import.parent_scope.module, key, false, |_, resolution| {
this.update_resolution(module, key, false, |_, resolution| {
resolution.single_imports.swap_remove(&import);
})
}
});
self.record_use(target, dummy_binding, Used::Other);
} else if import.imported_module.get().is_none() {
@@ -895,7 +907,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
PendingBinding::Ready(Some(imported_binding))
}
Err(Determinacy::Determined) => {
// Don't update the resolution for underscores, because it was never added.
// Don't remove underscores from `single_imports`, they were never added.
if target.name != kw::Underscore {
let key = BindingKey::new(target, ns);
this.update_resolution(parent, key, false, |_, resolution| {
@@ -1510,7 +1522,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
.is_some_and(|binding| binding.warn_ambiguity_recursive());
let _ = self.try_define(
import.parent_scope.module,
key,
key.ident,
key.ns,
imported_binding,
warn_ambiguity,
);

36
compiler/rustc_resolve/src/lib.rs
View File

@@ -532,15 +532,26 @@ struct BindingKey {
/// identifier.
ident: Ident,
ns: Namespace,
/// 0 if ident is not `_`, otherwise a value that's unique to the specific
/// `_` in the expanded AST that introduced this binding.
/// When we add an underscore binding (with ident `_`) to some module, this field has
/// a non-zero value that uniquely identifies this binding in that module.
/// For non-underscore bindings this field is zero.
/// When a key is constructed for name lookup (as opposed to name definition), this field is
/// also zero, even for underscore names, so for underscores the lookup will never succeed.
disambiguator: u32,
}
impl BindingKey {
fn new(ident: Ident, ns: Namespace) -> Self {
let ident = ident.normalize_to_macros_2_0();
BindingKey { ident, ns, disambiguator: 0 }
BindingKey { ident: ident.normalize_to_macros_2_0(), ns, disambiguator: 0 }
}
fn new_disambiguated(
ident: Ident,
ns: Namespace,
disambiguator: impl FnOnce() -> u32,
) -> BindingKey {
let disambiguator = if ident.name == kw::Underscore { disambiguator() } else { 0 };
BindingKey { ident: ident.normalize_to_macros_2_0(), ns, disambiguator }
}
}
@@ -568,6 +579,8 @@ struct ModuleData<'ra> {
lazy_resolutions: Resolutions<'ra>,
/// True if this is a module from other crate that needs to be populated on access.
populate_on_access: Cell<bool>,
/// Used to disambiguate underscore items (`const _: T = ...`) in the module.
underscore_disambiguator: Cell<u32>,
/// Macro invocations that can expand into items in this module.
unexpanded_invocations: RefCell<FxHashSet<LocalExpnId>>,
@@ -628,6 +641,7 @@ impl<'ra> ModuleData<'ra> {
kind,
lazy_resolutions: Default::default(),
populate_on_access: Cell::new(is_foreign),
underscore_disambiguator: Cell::new(0),
unexpanded_invocations: Default::default(),
no_implicit_prelude,
glob_importers: RefCell::new(Vec::new()),
@@ -1087,8 +1101,6 @@ pub struct Resolver<'ra, 'tcx> {
extern_module_map: RefCell<FxIndexMap<DefId, Module<'ra>>>,
binding_parent_modules: FxHashMap<NameBinding<'ra>, Module<'ra>>,
underscore_disambiguator: u32,
/// Maps glob imports to the names of items actually imported.
glob_map: FxIndexMap<LocalDefId, FxIndexSet<Symbol>>,
glob_error: Option<ErrorGuaranteed>,
@@ -1500,7 +1512,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
extern_crate_map: Default::default(),
module_children: Default::default(),
trait_map: NodeMap::default(),
underscore_disambiguator: 0,
empty_module,
local_module_map,
extern_module_map: Default::default(),
@@ -1881,17 +1892,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
import_ids
}
fn new_disambiguated_key(&mut self, ident: Ident, ns: Namespace) -> BindingKey {
let ident = ident.normalize_to_macros_2_0();
let disambiguator = if ident.name == kw::Underscore {
self.underscore_disambiguator += 1;
self.underscore_disambiguator
} else {
0
};
BindingKey { ident, ns, disambiguator }
}
fn resolutions(&mut self, module: Module<'ra>) -> &'ra Resolutions<'ra> {
if module.populate_on_access.get() {
module.populate_on_access.set(false);

2
compiler/rustc_resolve/src/macros.rs
View File

@@ -535,7 +535,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
target_trait.for_each_child(self, |this, ident, ns, _binding| {
// FIXME: Adjust hygiene for idents from globs, like for glob imports.
if let Some(overriding_keys) = this.impl_binding_keys.get(&impl_def_id)
&& overriding_keys.contains(&BindingKey::new(ident.normalize_to_macros_2_0(), ns))
&& overriding_keys.contains(&BindingKey::new(ident, ns))
{
// The name is overridden, do not produce it from the glob delegation.
} else {