Skip to content

resolve: Do not create NameResolutions on access unless necessary #144468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,10 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
self.r.per_ns(|this, ns| {
if !type_ns_only || ns == TypeNS {
let key = BindingKey::new(target, ns);
let mut resolution = this.resolution(current_module, key).borrow_mut();
resolution.single_imports.insert(import);
this.resolution_or_default(current_module, key)
.borrow_mut()
.single_imports
.insert(import);
}
});
}
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_resolve/src/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,9 +509,7 @@ impl Resolver<'_, '_> {
let mut check_redundant_imports = FxIndexSet::default();
for module in self.arenas.local_modules().iter() {
for (_key, resolution) in self.resolutions(*module).borrow().iter() {
let resolution = resolution.borrow();

if let Some(binding) = resolution.best_binding()
if let Some(binding) = resolution.borrow().best_binding()
&& let NameBindingKind::Import { import, .. } = binding.kind
&& let ImportKind::Single { id, .. } = import.kind
{
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2659,10 +2659,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
return None;
}

let resolutions = self.resolutions(crate_module).borrow();
let binding_key = BindingKey::new(ident, MacroNS);
let resolution = resolutions.get(&binding_key)?;
let binding = resolution.borrow().binding()?;
let binding = self.resolution(crate_module, binding_key)?.binding()?;
let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() else {
return None;
};
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_resolve/src/effective_visibilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,7 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
/// including their whole reexport chains.
fn set_bindings_effective_visibilities(&mut self, module_id: LocalDefId) {
let module = self.r.expect_module(module_id.to_def_id());
let resolutions = self.r.resolutions(module);

for (_, name_resolution) in resolutions.borrow().iter() {
for (_, name_resolution) in self.r.resolutions(module).borrow().iter() {
let Some(mut binding) = name_resolution.borrow().binding() else {
continue;
};
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -848,8 +848,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
};

let key = BindingKey::new(ident, ns);
let resolution =
self.resolution(module, key).try_borrow_mut().map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports.
// `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
// the exclusive access infinite recursion will crash the compiler with stack overflow.
let resolution = &*self
.resolution_or_default(module, key)
.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
Expand Down
68 changes: 32 additions & 36 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// Ensure that `resolution` isn't borrowed when defining in the module's glob importers,
// during which the resolution might end up getting re-defined via a glob cycle.
let (binding, t, warn_ambiguity) = {
let resolution = &mut *self.resolution(module, key).borrow_mut();
let resolution = &mut *self.resolution_or_default(module, key).borrow_mut();
let old_binding = resolution.binding();

let t = f(self, resolution);
Expand Down Expand Up @@ -651,7 +651,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
for module in self.arenas.local_modules().iter() {
for (key, resolution) in self.resolutions(*module).borrow().iter() {
let resolution = resolution.borrow();

let Some(binding) = resolution.best_binding() else { continue };

if let NameBindingKind::Import { import, .. } = binding.kind
Expand Down Expand Up @@ -1202,41 +1201,39 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
});

return if all_ns_failed {
let resolutions = match module {
ModuleOrUniformRoot::Module(module) => Some(self.resolutions(module).borrow()),
_ => None,
};
let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter());
let names = resolutions
.filter_map(|(BindingKey { ident: i, .. }, resolution)| {
if i.name == ident.name {
return None;
} // Never suggest the same name
match *resolution.borrow() {
ref resolution
if let Some(name_binding) = resolution.best_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),
let names = match module {
ModuleOrUniformRoot::Module(module) => {
self.resolutions(module)
.borrow()
.iter()
.filter_map(|(BindingKey { ident: i, .. }, resolution)| {
if i.name == ident.name {
return None;
} // Never suggest the same name

let resolution = resolution.borrow();
if let Some(name_binding) = resolution.best_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),
} else if resolution.single_imports.is_empty() {
None
} else {
Some(i.name)
}
}
NameResolution { ref single_imports, .. }
if single_imports.is_empty() =>
{
None
}
_ => Some(i.name),
}
})
.collect::<Vec<Symbol>>();
})
.collect()
}
_ => Vec::new(),
};

let lev_suggestion =
find_best_match_for_name(&names, ident.name, None).map(|suggestion| {
Expand Down Expand Up @@ -1517,8 +1514,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
let imported_binding = self.import(binding, import);
let warn_ambiguity = self
.resolution(import.parent_scope.module, key)
.borrow()
.binding()
.and_then(|r| r.binding())
.is_some_and(|binding| binding.warn_ambiguity_recursive());
let _ = self.try_define(
import.parent_scope.module,
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3449,8 +3449,7 @@ 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.best_binding());
let mut binding = self.r.resolution(module, key).and_then(|r| r.best_binding());
debug!(?binding);
if binding.is_none() {
// We could not find the trait item in the correct namespace.
Expand All @@ -3461,8 +3460,7 @@ 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.best_binding());
binding = self.r.resolution(module, key).and_then(|r| r.best_binding());
debug!(?binding);
}

Expand Down
13 changes: 8 additions & 5 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1461,15 +1461,17 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
if let PathResult::Module(ModuleOrUniformRoot::Module(module)) =
self.resolve_path(mod_path, None, None)
{
let resolutions = self.r.resolutions(module).borrow();
let targets: Vec<_> = resolutions
let targets: Vec<_> = self
.r
.resolutions(module)
.borrow()
.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 })
.and_then(|res| if filter_fn(res) { Some((*key, res)) } else { None })
})
.collect();
if let [target] = targets.as_slice() {
Expand Down Expand Up @@ -2300,8 +2302,9 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
return None;
}

let resolutions = self.r.resolutions(*module);
let targets = resolutions
let targets = self
.r
.resolutions(*module)
.borrow()
.iter()
.filter_map(|(key, res)| {
Expand Down
13 changes: 10 additions & 3 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#![recursion_limit = "256"]
// tidy-alphabetical-end

use std::cell::{Cell, RefCell};
use std::cell::{Cell, Ref, RefCell};
use std::collections::BTreeSet;
use std::fmt;
use std::sync::Arc;
Expand Down Expand Up @@ -1905,9 +1905,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
&mut self,
module: Module<'ra>,
key: BindingKey,
) -> Option<Ref<'ra, NameResolution<'ra>>> {
self.resolutions(module).borrow().get(&key).map(|resolution| resolution.borrow())
}

fn resolution_or_default(
&mut self,
module: Module<'ra>,
key: BindingKey,
) -> &'ra RefCell<NameResolution<'ra>> {
*self
.resolutions(module)
self.resolutions(module)
.borrow_mut()
.entry(key)
.or_insert_with(|| self.arenas.alloc_name_resolution())
Expand Down
Loading