Skip to content

Commit 7773a3f

Browse files
Auto merge of #144468 - petrochenkov:resolution, r=<try>
resolve: Do not create `NameResolutions` on access unless necessary
2 parents a955f1c + e820112 commit 7773a3f

File tree

9 files changed

+66
-61
lines changed

9 files changed

+66
-61
lines changed

compiler/rustc_resolve/src/build_reduced_graph.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -452,8 +452,10 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
452452
self.r.per_ns(|this, ns| {
453453
if !type_ns_only || ns == TypeNS {
454454
let key = BindingKey::new(target, ns);
455-
let mut resolution = this.resolution(current_module, key).borrow_mut();
456-
resolution.single_imports.insert(import);
455+
this.resolution_or_default(current_module, key)
456+
.borrow_mut()
457+
.single_imports
458+
.insert(import);
457459
}
458460
});
459461
}

compiler/rustc_resolve/src/check_unused.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -509,9 +509,7 @@ impl Resolver<'_, '_> {
509509
let mut check_redundant_imports = FxIndexSet::default();
510510
for module in self.arenas.local_modules().iter() {
511511
for (_key, resolution) in self.resolutions(*module).borrow().iter() {
512-
let resolution = resolution.borrow();
513-
514-
if let Some(binding) = resolution.best_binding()
512+
if let Some(binding) = resolution.borrow().best_binding()
515513
&& let NameBindingKind::Import { import, .. } = binding.kind
516514
&& let ImportKind::Single { id, .. } = import.kind
517515
{

compiler/rustc_resolve/src/diagnostics.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2659,10 +2659,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
26592659
return None;
26602660
}
26612661

2662-
let resolutions = self.resolutions(crate_module).borrow();
26632662
let binding_key = BindingKey::new(ident, MacroNS);
2664-
let resolution = resolutions.get(&binding_key)?;
2665-
let binding = resolution.borrow().binding()?;
2663+
let binding = self.resolution(crate_module, binding_key)?.binding()?;
26662664
let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() else {
26672665
return None;
26682666
};

compiler/rustc_resolve/src/effective_visibilities.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,7 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
114114
/// including their whole reexport chains.
115115
fn set_bindings_effective_visibilities(&mut self, module_id: LocalDefId) {
116116
let module = self.r.expect_module(module_id.to_def_id());
117-
let resolutions = self.r.resolutions(module);
118-
119-
for (_, name_resolution) in resolutions.borrow().iter() {
117+
for (_, name_resolution) in self.r.resolutions(module).borrow().iter() {
120118
let Some(mut binding) = name_resolution.borrow().binding() else {
121119
continue;
122120
};

compiler/rustc_resolve/src/ident.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -848,8 +848,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
848848
};
849849

850850
let key = BindingKey::new(ident, ns);
851-
let resolution =
852-
self.resolution(module, key).try_borrow_mut().map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports.
851+
// `try_borrow_mut` is required to ensure exclusive access, even if the resulting binding
852+
// doesn't need to be mutable. It will fail when there is a cycle of imports, and without
853+
// the exclusive access infinite recursion will crash the compiler with stack overflow.
854+
let resolution = &*self
855+
.resolution_or_default(module, key)
856+
.try_borrow_mut()
857+
.map_err(|_| (Determined, Weak::No))?;
853858

854859
// If the primary binding is unusable, search further and return the shadowed glob
855860
// binding if it exists. What we really want here is having two separate scopes in

compiler/rustc_resolve/src/imports.rs

Lines changed: 32 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
469469
// Ensure that `resolution` isn't borrowed when defining in the module's glob importers,
470470
// during which the resolution might end up getting re-defined via a glob cycle.
471471
let (binding, t, warn_ambiguity) = {
472-
let resolution = &mut *self.resolution(module, key).borrow_mut();
472+
let resolution = &mut *self.resolution_or_default(module, key).borrow_mut();
473473
let old_binding = resolution.binding();
474474

475475
let t = f(self, resolution);
@@ -651,7 +651,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
651651
for module in self.arenas.local_modules().iter() {
652652
for (key, resolution) in self.resolutions(*module).borrow().iter() {
653653
let resolution = resolution.borrow();
654-
655654
let Some(binding) = resolution.best_binding() else { continue };
656655

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

12041203
return if all_ns_failed {
1205-
let resolutions = match module {
1206-
ModuleOrUniformRoot::Module(module) => Some(self.resolutions(module).borrow()),
1207-
_ => None,
1208-
};
1209-
let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter());
1210-
let names = resolutions
1211-
.filter_map(|(BindingKey { ident: i, .. }, resolution)| {
1212-
if i.name == ident.name {
1213-
return None;
1214-
} // Never suggest the same name
1215-
match *resolution.borrow() {
1216-
ref resolution
1217-
if let Some(name_binding) = resolution.best_binding() =>
1218-
{
1219-
match name_binding.kind {
1220-
NameBindingKind::Import { binding, .. } => {
1221-
match binding.kind {
1222-
// Never suggest the name that has binding error
1223-
// i.e., the name that cannot be previously resolved
1224-
NameBindingKind::Res(Res::Err) => None,
1225-
_ => Some(i.name),
1204+
let names = match module {
1205+
ModuleOrUniformRoot::Module(module) => {
1206+
self.resolutions(module)
1207+
.borrow()
1208+
.iter()
1209+
.filter_map(|(BindingKey { ident: i, .. }, resolution)| {
1210+
if i.name == ident.name {
1211+
return None;
1212+
} // Never suggest the same name
1213+
1214+
let resolution = resolution.borrow();
1215+
if let Some(name_binding) = resolution.best_binding() {
1216+
match name_binding.kind {
1217+
NameBindingKind::Import { binding, .. } => {
1218+
match binding.kind {
1219+
// Never suggest the name that has binding error
1220+
// i.e., the name that cannot be previously resolved
1221+
NameBindingKind::Res(Res::Err) => None,
1222+
_ => Some(i.name),
1223+
}
12261224
}
1225+
_ => Some(i.name),
12271226
}
1228-
_ => Some(i.name),
1227+
} else if resolution.single_imports.is_empty() {
1228+
None
1229+
} else {
1230+
Some(i.name)
12291231
}
1230-
}
1231-
NameResolution { ref single_imports, .. }
1232-
if single_imports.is_empty() =>
1233-
{
1234-
None
1235-
}
1236-
_ => Some(i.name),
1237-
}
1238-
})
1239-
.collect::<Vec<Symbol>>();
1232+
})
1233+
.collect()
1234+
}
1235+
_ => Vec::new(),
1236+
};
12401237

12411238
let lev_suggestion =
12421239
find_best_match_for_name(&names, ident.name, None).map(|suggestion| {
@@ -1517,8 +1514,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
15171514
let imported_binding = self.import(binding, import);
15181515
let warn_ambiguity = self
15191516
.resolution(import.parent_scope.module, key)
1520-
.borrow()
1521-
.binding()
1517+
.and_then(|r| r.binding())
15221518
.is_some_and(|binding| binding.warn_ambiguity_recursive());
15231519
let _ = self.try_define(
15241520
import.parent_scope.module,

compiler/rustc_resolve/src/late.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3449,8 +3449,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
34493449
};
34503450
ident.span.normalize_to_macros_2_0_and_adjust(module.expansion);
34513451
let key = BindingKey::new(ident, ns);
3452-
let mut binding =
3453-
self.r.resolution(module, key).try_borrow().ok().and_then(|r| r.best_binding());
3452+
let mut binding = self.r.resolution(module, key).and_then(|r| r.best_binding());
34543453
debug!(?binding);
34553454
if binding.is_none() {
34563455
// We could not find the trait item in the correct namespace.
@@ -3461,8 +3460,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
34613460
_ => ns,
34623461
};
34633462
let key = BindingKey::new(ident, ns);
3464-
binding =
3465-
self.r.resolution(module, key).try_borrow().ok().and_then(|r| r.best_binding());
3463+
binding = self.r.resolution(module, key).and_then(|r| r.best_binding());
34663464
debug!(?binding);
34673465
}
34683466

compiler/rustc_resolve/src/late/diagnostics.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,15 +1461,17 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
14611461
if let PathResult::Module(ModuleOrUniformRoot::Module(module)) =
14621462
self.resolve_path(mod_path, None, None)
14631463
{
1464-
let resolutions = self.r.resolutions(module).borrow();
1465-
let targets: Vec<_> = resolutions
1464+
let targets: Vec<_> = self
1465+
.r
1466+
.resolutions(module)
1467+
.borrow()
14661468
.iter()
14671469
.filter_map(|(key, resolution)| {
14681470
resolution
14691471
.borrow()
14701472
.best_binding()
14711473
.map(|binding| binding.res())
1472-
.and_then(|res| if filter_fn(res) { Some((key, res)) } else { None })
1474+
.and_then(|res| if filter_fn(res) { Some((*key, res)) } else { None })
14731475
})
14741476
.collect();
14751477
if let [target] = targets.as_slice() {
@@ -2300,8 +2302,9 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
23002302
return None;
23012303
}
23022304

2303-
let resolutions = self.r.resolutions(*module);
2304-
let targets = resolutions
2305+
let targets = self
2306+
.r
2307+
.resolutions(*module)
23052308
.borrow()
23062309
.iter()
23072310
.filter_map(|(key, res)| {

compiler/rustc_resolve/src/lib.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
#![recursion_limit = "256"]
2222
// tidy-alphabetical-end
2323

24-
use std::cell::{Cell, RefCell};
24+
use std::cell::{Cell, Ref, RefCell};
2525
use std::collections::BTreeSet;
2626
use std::fmt;
2727
use std::sync::Arc;
@@ -1905,9 +1905,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
19051905
&mut self,
19061906
module: Module<'ra>,
19071907
key: BindingKey,
1908+
) -> Option<Ref<'ra, NameResolution<'ra>>> {
1909+
self.resolutions(module).borrow().get(&key).map(|resolution| resolution.borrow())
1910+
}
1911+
1912+
fn resolution_or_default(
1913+
&mut self,
1914+
module: Module<'ra>,
1915+
key: BindingKey,
19081916
) -> &'ra RefCell<NameResolution<'ra>> {
1909-
*self
1910-
.resolutions(module)
1917+
self.resolutions(module)
19111918
.borrow_mut()
19121919
.entry(key)
19131920
.or_insert_with(|| self.arenas.alloc_name_resolution())

0 commit comments

Comments
 (0)