Recursion stack for detecting cyclic imports

compiler/rustc_resolve/src/ident.rs

@@ -1094,13 +1094,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         ignore_import: Option<Import<'ra>>,
     ) -> Result<Decl<'ra>, ControlFlow<Determinacy, Determinacy>> {
         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
-        // the exclusive access infinite recursion will crash the compiler with stack overflow.
-        let resolution = &*self
-            .resolution_or_default(module, key, orig_ident_span)
-            .try_borrow_mut_unchecked()
-            .map_err(|_| ControlFlow::Continue(Determined))?;
+        let resolution = self.resolution_or_default(module, key, orig_ident_span);
+        // We need to detect import cycles to avoid infinite recursion. The guard ensures
+        // the resolution is removed when this resolve call ends.
+        let _cycle_guard =
+            self.enter_cycle_detector(resolution).map_err(|_| ControlFlow::Continue(Determined))?;
+        let resolution = resolution.borrow();
 
         let binding = resolution.non_glob_decl.filter(|b| Some(*b) != ignore_decl);
 
@@ -1161,13 +1160,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         ignore_import: Option<Import<'ra>>,
     ) -> Result<Decl<'ra>, ControlFlow<Determinacy, Determinacy>> {
         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
-        // the exclusive access infinite recursion will crash the compiler with stack overflow.
-        let resolution = &*self
-            .resolution_or_default(module.to_module(), key, orig_ident_span)
-            .try_borrow_mut_unchecked()
-            .map_err(|_| ControlFlow::Continue(Determined))?;
+        let resolution = self.resolution_or_default(module.to_module(), key, orig_ident_span);
+        // We need to detect import cycles to avoid infinite recursion. The guard ensures
+        // the resolution is removed when this resolve call ends.
+        let _cycle_guard =
+            self.enter_cycle_detector(resolution).map_err(|_| ControlFlow::Continue(Determined))?;
+        let resolution = resolution.borrow();
 
         let binding = resolution.glob_decl.filter(|b| Some(*b) != ignore_decl);
 

compiler/rustc_resolve/src/imports.rs

@@ -350,6 +350,60 @@ impl<'ra> NameResolution<'ra> {
     }
 }
 
+// module to keep the TLS private and only accessible through the `Resolver`.
+pub(crate) mod cycle_detection {
+    use std::collections::BTreeSet;
+
+    use crate::imports::NameResolution;
+    use crate::ref_mut::CmRefCell;
+    use crate::{CacheRefCell, Resolver};
+
+    thread_local! {
+        /// During import resolution, recursive imports can form cycles.
+        /// This set stores the active resolution stack for the current thread.
+        /// So it's essentially a recursion stack.
+        ///
+        /// The key is the address of a `NameResolution`; we only need identity,
+        /// not access to the value.
+        static ACTIVE_RESOLUTIONS: CacheRefCell<BTreeSet<*const ()>> = Default::default();
+    }
+
+    /// We need a way to remove the resolution from the stack whenever we return, this type
+    /// holds that resolution key and removes it on drop.
+    pub(crate) struct ImportCycleGuard {
+        key: *const (),
+    }
+
+    impl Drop for ImportCycleGuard {
+        fn drop(&mut self) {
+            ACTIVE_RESOLUTIONS.with_borrow_mut(|active| {
+                active.remove(&self.key);
+            });
+        }
+    }
+
+    impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
+        /// Enters the `resolution` into the active import resolution stack.
+        ///
+        /// Returns `Err(())` if `res` is already active, which means an import cycle
+        /// was detected.
+        pub(crate) fn enter_cycle_detector(
+            &self,
+            resolution: &'ra CmRefCell<NameResolution<'ra>>,
+        ) -> Result<ImportCycleGuard, ()> {
+            let key = std::ptr::from_ref(resolution).cast::<()>();
+
+            ACTIVE_RESOLUTIONS.with_borrow_mut(|active| {
+                if !active.insert(key) {
+                    return Err(());
+                }
+
+                Ok(ImportCycleGuard { key })
+            })
+        }
+    }
+}
+
 /// An error that may be transformed into a diagnostic later. Used to combine multiple unresolved
 /// import errors within the same use tree into a single diagnostic.
 #[derive(Debug, Clone)]

compiler/rustc_resolve/src/lib.rs

@@ -2944,10 +2944,6 @@ mod ref_mut {
             self.0.borrow_mut()
         }
 
-        pub(crate) fn try_borrow_mut_unchecked(&self) -> Result<RefMut<'_, T>, BorrowMutError> {
-            self.0.try_borrow_mut()
-        }
-
         #[track_caller]
         pub(crate) fn try_borrow_mut<'ra, 'tcx>(
             &self,