authorbors <bors@rust-lang.org> 2026-06-29 18:35:05 UTC
committerbors <bors@rust-lang.org> 2026-06-29 18:35:05 UTC
log096694416a41840709140eb0fd0ca193d1a3e6ba
treef199e7547bd2d30d35cad293c7ccca12e2bf9d3a
parent7dc2c162b9c197aaa76a6f9e7534569537830a01
parent3056b8e4ec9fe9498be410e35308f3ba673bba6d

Auto merge of #158035 - LorrensP-2158466:import-cycle-det, r=petrochenkov

resolve: Explicit Set for detecting resolution cycles Instead of using the `borrow_mut` counter of a `RefCell` for a `NameResolution` for detecting cyclic imports during import resolution, we use an explicit recursion stack that keeps track of the current used `NameResolution`s. Because of the upcoming parallelisation of the import resolution algorithm, the current way cannot used in a parallel context. r? @petrochenkov

3 files changed, 82 insertions(+), 31 deletions(-)

compiler/rustc_resolve/src/ident.rs+15-15
......@@ -16,7 +16,7 @@ use tracing::{debug, instrument};
1616
1717use crate::diagnostics::{ParamKindInEnumDiscriminant, ParamKindInNonTrivialAnonConst};
1818use crate::hygiene::Macros20NormalizedSyntaxContext;
19use crate::imports::{Import, NameResolution};
19use crate::imports::{Import, NameResolution, cycle_detection};
2020use crate::late::{
2121 ConstantHasGenerics, DiagMetadata, NoConstantGenericsReason, PathSource, Rib, RibKind,
2222};
......@@ -1148,13 +1148,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
11481148 ignore_import: Option<Import<'ra>>,
11491149 ) -> Result<Decl<'ra>, ControlFlow<Determinacy, Determinacy>> {
11501150 let key = BindingKey::new(ident, ns);
1151 // `try_borrow_mut` is required to ensure exclusive access, even if the resulting binding
1152 // doesn't need to be mutable. It will fail when there is a cycle of imports, and without
1153 // the exclusive access infinite recursion will crash the compiler with stack overflow.
1154 let resolution = &*self
1155 .resolution_or_default(module.to_module(), key, orig_ident_span)
1156 .try_borrow_mut_unchecked()
1157 .map_err(|_| ControlFlow::Continue(Determined))?;
1151 let resolution_ref = self.resolution_or_default(module.to_module(), key, orig_ident_span);
1152 let resolution = resolution_ref.borrow();
11581153
11591154 let binding = resolution.non_glob_decl.filter(|b| Some(*b) != ignore_decl);
11601155
......@@ -1175,6 +1170,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
11751170 return if accessible { Ok(binding) } else { Err(ControlFlow::Break(Determined)) };
11761171 }
11771172
1173 // We need to detect resolution cycles to avoid infinite recursion. The guard ensures
1174 // the resolution is removed when this resolve call ends.
1175 let _cycle_guard = cycle_detection::enter_cycle_detector(resolution_ref)
1176 .map_err(|_| ControlFlow::Continue(Determined))?;
1177
11781178 // Check if one of single imports can still define the name, block if it can.
11791179 if self.reborrow().single_import_can_define_name(
11801180 &resolution,
......@@ -1210,13 +1210,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
12101210 ignore_import: Option<Import<'ra>>,
12111211 ) -> Result<Decl<'ra>, ControlFlow<Determinacy, Determinacy>> {
12121212 let key = BindingKey::new(ident, ns);
1213 // `try_borrow_mut` is required to ensure exclusive access, even if the resulting binding
1214 // doesn't need to be mutable. It will fail when there is a cycle of imports, and without
1215 // the exclusive access infinite recursion will crash the compiler with stack overflow.
1216 let resolution = &*self
1217 .resolution_or_default(module.to_module(), key, orig_ident_span)
1218 .try_borrow_mut_unchecked()
1219 .map_err(|_| ControlFlow::Continue(Determined))?;
1213 let resolution_ref = self.resolution_or_default(module.to_module(), key, orig_ident_span);
1214 let resolution = resolution_ref.borrow();
12201215
12211216 let binding = resolution.glob_decl.filter(|b| Some(*b) != ignore_decl);
12221217
......@@ -1231,6 +1226,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
12311226 );
12321227 }
12331228
1229 // We need to detect resolution cycles to avoid infinite recursion. The guard ensures
1230 // the resolution is removed when this resolve call ends.
1231 let _cycle_guard = cycle_detection::enter_cycle_detector(resolution_ref)
1232 .map_err(|_| ControlFlow::Continue(Determined))?;
1233
12341234 // Check if one of single imports can still define the name,
12351235 // if it can then our result is not determined and can be invalidated.
12361236 if self.reborrow().single_import_can_define_name(
compiler/rustc_resolve/src/imports.rs+55-1
......@@ -32,7 +32,7 @@ use crate::diagnostics::{
3232 ConsiderMarkingAsPubCrate,
3333};
3434use crate::error_helper::{OnUnknownData, Suggestion};
35use crate::ref_mut::CmCell;
35use crate::ref_mut::{CmCell, CmRefCell};
3636use crate::{
3737 AmbiguityError, BindingKey, CmResolver, Decl, DeclData, DeclKind, Determinacy, Finalize,
3838 IdentKey, ImportSuggestion, ImportSummary, LocalModule, ModuleOrUniformRoot, ParentScope,
......@@ -292,6 +292,8 @@ pub(crate) struct NameResolution<'ra> {
292292 pub orig_ident_span: Span,
293293}
294294
295pub(crate) type NameResolutionRef<'ra> = Interned<'ra, CmRefCell<NameResolution<'ra>>>;
296
295297impl<'ra> NameResolution<'ra> {
296298 pub(crate) fn new(orig_ident_span: Span) -> Self {
297299 NameResolution { single_imports: FxIndexSet::default(), orig_ident_span, .. }
......@@ -320,6 +322,57 @@ impl<'ra> NameResolution<'ra> {
320322 }
321323}
322324
325// module to keep the TLS private and only accessible through the function `enter_cycle_detector`.
326pub(crate) mod cycle_detection {
327 use std::ptr;
328
329 use crate::CacheRefCell;
330 use crate::imports::NameResolutionRef;
331
332 thread_local!(
333 /// During import resolution, recursive imports can form cycles.
334 /// This set stores the active resolution stack for the current thread.
335 /// So it's essentially a recursion stack.
336 ///
337 /// The key is the interned address of a `RefCell<NameResolution<'ra>>` allocated
338 /// in the `Resolver Arenas` (lifetime `'ra`), it is thus stable and allows casting
339 /// to a `*const ()` for comparison. This is done because we can't use lifetimes
340 /// other than `'static` in thread local storage.
341 static ACTIVE_RESOLUTIONS: CacheRefCell<Vec<*const ()>> = Default::default();
342 );
343
344 pub(crate) struct ActiveResolutionGuard {
345 key: *const (),
346 }
347
348 impl Drop for ActiveResolutionGuard {
349 fn drop(&mut self) {
350 ACTIVE_RESOLUTIONS.with_borrow_mut(|ar| {
351 // Only this guard is allowed to remove this key.
352 assert!(
353 Some(self.key) == ar.pop(),
354 "This guard should be the only one removing this key"
355 );
356 });
357 }
358 }
359
360 /// Returns `Err(())` if a cycle is detected, otherwise this returns a
361 /// guard that will remove the resolution when dropped.
362 pub(crate) fn enter_cycle_detector<'ra>(
363 resolution: NameResolutionRef<'ra>,
364 ) -> Result<ActiveResolutionGuard, ()> {
365 let key = ptr::from_ref(resolution.0).cast::<()>();
366 ACTIVE_RESOLUTIONS.with_borrow_mut(|ar| {
367 if ar.contains(&key) {
368 return Err(());
369 }
370 ar.push(key);
371 Ok(ActiveResolutionGuard { key })
372 })
373 }
374}
375
323376/// An error that may be transformed into a diagnostic later. Used to combine multiple unresolved
324377/// import errors within the same use tree into a single diagnostic.
325378#[derive(Debug, Clone)]
......@@ -640,6 +693,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
640693 let (binding, t) = {
641694 let resolution = &mut *self
642695 .resolution_or_default(module.to_module(), key, orig_ident_span)
696 .0
643697 .borrow_mut(self);
644698 let old_decl = resolution.determined_decl();
645699 let old_vis = old_decl.map(|d| d.vis());
compiler/rustc_resolve/src/lib.rs+12-15
......@@ -77,6 +77,7 @@ use smallvec::{SmallVec, smallvec};
7777use tracing::{debug, instrument};
7878
7979use crate::error_helper::OnUnknownData;
80use crate::imports::NameResolutionRef;
8081use crate::ref_mut::{CmCell, CmRefCell};
8182
8283mod build_reduced_graph;
......@@ -639,7 +640,7 @@ impl BindingKey {
639640 }
640641}
641642
642type Resolutions<'ra> = CmRefCell<FxIndexMap<BindingKey, &'ra CmRefCell<NameResolution<'ra>>>>;
643type Resolutions<'ra> = CmRefCell<FxIndexMap<BindingKey, NameResolutionRef<'ra>>>;
643644
644645/// One node in the tree of modules.
645646///
......@@ -1597,11 +1598,10 @@ impl<'ra> ResolverArenas<'ra> {
15971598 fn alloc_import(&'ra self, import: ImportData<'ra>) -> Import<'ra> {
15981599 Interned::new_unchecked(self.imports.alloc(import))
15991600 }
1600 fn alloc_name_resolution(
1601 &'ra self,
1602 orig_ident_span: Span,
1603 ) -> &'ra CmRefCell<NameResolution<'ra>> {
1604 self.name_resolutions.alloc(CmRefCell::new(NameResolution::new(orig_ident_span)))
1601 fn alloc_name_resolution(&'ra self, orig_ident_span: Span) -> NameResolutionRef<'ra> {
1602 Interned::new_unchecked(
1603 self.name_resolutions.alloc(CmRefCell::new(NameResolution::new(orig_ident_span))),
1604 )
16051605 }
16061606 fn alloc_macro_rules_scope(&'ra self, scope: MacroRulesScope<'ra>) -> MacroRulesScopeRef<'ra> {
16071607 self.dropless.alloc(CacheCell::new(scope))
......@@ -2213,7 +2213,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
22132213 module: Module<'ra>,
22142214 key: BindingKey,
22152215 ) -> Option<Ref<'ra, NameResolution<'ra>>> {
2216 self.resolutions(module).borrow().get(&key).map(|resolution| resolution.borrow())
2216 self.resolutions(module).borrow().get(&key).map(|resolution| resolution.0.borrow())
22172217 }
22182218
22192219 fn resolution_or_default(
......@@ -2221,8 +2221,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
22212221 module: Module<'ra>,
22222222 key: BindingKey,
22232223 orig_ident_span: Span,
2224 ) -> &'ra CmRefCell<NameResolution<'ra>> {
2225 self.resolutions(module)
2224 ) -> NameResolutionRef<'ra> {
2225 *self
2226 .resolutions(module)
22262227 .borrow_mut_unchecked()
22272228 .entry(key)
22282229 .or_insert_with(|| self.arenas.alloc_name_resolution(orig_ident_span))
......@@ -2833,8 +2834,6 @@ type CmResolver<'r, 'ra, 'tcx> = ref_mut::RefOrMut<'r, Resolver<'ra, 'tcx>>;
28332834// parallel name resolution.
28342835use std::cell::{Cell as CacheCell, RefCell as CacheRefCell};
28352836
2836// FIXME: `*_unchecked` methods in the module below should be eliminated in the process
2837// of migration to parallel name resolution.
28382837mod ref_mut {
28392838 use std::cell::{BorrowMutError, Cell, Ref, RefCell, RefMut};
28402839 use std::fmt;
......@@ -2942,6 +2941,8 @@ mod ref_mut {
29422941 }
29432942
29442943 #[track_caller]
2944 // FIXME: this should be eliminated in the process of migration
2945 // to parallel name resolution.
29452946 pub(crate) fn borrow_mut_unchecked(&self) -> RefMut<'_, T> {
29462947 self.0.borrow_mut()
29472948 }
......@@ -2954,10 +2955,6 @@ mod ref_mut {
29542955 self.0.borrow_mut()
29552956 }
29562957
2957 pub(crate) fn try_borrow_mut_unchecked(&self) -> Result<RefMut<'_, T>, BorrowMutError> {
2958 self.0.try_borrow_mut()
2959 }
2960
29612958 #[track_caller]
29622959 pub(crate) fn try_borrow_mut<'ra, 'tcx>(
29632960 &self,