resolve: Use primitives for conditional mutability more consistently

petrochenkov · petrochenkov · commit 204508d9282c · 2025-10-09T23:10:25.000+03:00
diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -87,13 +87,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         // 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, || {
-            // FIXME(batched): Will be fixed in batched resolution.
             parent.underscore_disambiguator.update_unchecked(|d| d + 1);
             parent.underscore_disambiguator.get()
         });
         if self
             .resolution_or_default(parent, key)
-            .borrow_mut()
+            .borrow_mut_unchecked()
             .non_glob_binding
             .replace(binding)
             .is_some()
@@ -499,7 +498,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
                         if !type_ns_only || ns == TypeNS {
                             let key = BindingKey::new(target, ns);
                             this.resolution_or_default(current_module, key)
-                                .borrow_mut()
+                                .borrow_mut(this)
                                 .single_imports
                                 .insert(import);
                         }
diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs
@@ -891,7 +891,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         // the exclusive access infinite recursion will crash the compiler with stack overflow.
         let resolution = &*self
             .resolution_or_default(module, key)
-            .try_borrow_mut()
+            .try_borrow_mut_unchecked()
             .map_err(|_| (Determined, Weak::No))?;
 
         // If the primary binding is unusable, search further and return the shadowed glob
diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
@@ -320,7 +320,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             && (vis == import_vis
                 || max_vis.get().is_none_or(|max_vis| vis.is_at_least(max_vis, self.tcx)))
         {
-            // FIXME(batched): Will be fixed in batched import resolution.
             max_vis.set_unchecked(Some(vis.expect_local()))
         }
 
@@ -350,7 +349,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         // 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, || {
-            // FIXME(batched): Will be fixed in batched resolution.
             module.underscore_disambiguator.update_unchecked(|d| d + 1);
             module.underscore_disambiguator.get()
         });
@@ -470,7 +468,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_or_default(module, key).borrow_mut();
+            let resolution = &mut *self.resolution_or_default(module, key).borrow_mut_unchecked();
             let old_binding = resolution.binding();
 
             let t = f(self, resolution);
@@ -553,12 +551,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
     /// Resolves all imports for the crate. This method performs the fixed-
     /// point iteration.
     pub(crate) fn resolve_imports(&mut self) {
-        self.assert_speculative = true;
         let mut prev_indeterminate_count = usize::MAX;
         let mut indeterminate_count = self.indeterminate_imports.len() * 3;
         while indeterminate_count < prev_indeterminate_count {
             prev_indeterminate_count = indeterminate_count;
             indeterminate_count = 0;
+            self.assert_speculative = true;
             for import in mem::take(&mut self.indeterminate_imports) {
                 let import_indeterminate_count = self.cm().resolve_import(import);
                 indeterminate_count += import_indeterminate_count;
@@ -567,8 +565,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                     _ => self.indeterminate_imports.push(import),
                 }
             }
+            self.assert_speculative = false;
         }
-        self.assert_speculative = false;
     }
 
     pub(crate) fn finalize_imports(&mut self) {
@@ -862,15 +860,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             }
         };
 
-        // FIXME(batched): Will be fixed in batched import resolution.
         import.imported_module.set_unchecked(Some(module));
         let (source, target, bindings, type_ns_only) = match import.kind {
             ImportKind::Single { source, target, ref bindings, type_ns_only, .. } => {
                 (source, target, bindings, type_ns_only)
             }
             ImportKind::Glob { .. } => {
-                // FIXME: Use mutable resolver directly as a hack, this should be an output of
-                // speculative resolution.
                 self.get_mut_unchecked().resolve_glob_import(import);
                 return 0;
             }
@@ -906,8 +901,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         }
                         // We need the `target`, `source` can be extracted.
                         let imported_binding = this.import(binding, import);
-                        // FIXME: Use mutable resolver directly as a hack, this should be an output of
-                        // speculative resolution.
                         this.get_mut_unchecked().define_binding_local(
                             parent,
                             target,
@@ -920,8 +913,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         // Don't remove underscores from `single_imports`, they were never added.
                         if target.name != kw::Underscore {
                             let key = BindingKey::new(target, ns);
-                            // FIXME: Use mutable resolver directly as a hack, this should be an output of
-                            // speculative resolution.
                             this.get_mut_unchecked().update_local_resolution(
                                 parent,
                                 key,
@@ -938,7 +929,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         PendingBinding::Pending
                     }
                 };
-                // FIXME(batched): Will be fixed in batched import resolution.
                 bindings[ns].set_unchecked(binding);
             }
         });
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
@@ -25,7 +25,7 @@
 #![recursion_limit = "256"]
 // tidy-alphabetical-end
 
-use std::cell::{Cell, Ref, RefCell};
+use std::cell::Ref;
 use std::collections::BTreeSet;
 use std::fmt::{self};
 use std::sync::Arc;
@@ -572,7 +572,7 @@ impl BindingKey {
     }
 }
 
-type Resolutions<'ra> = RefCell<FxIndexMap<BindingKey, &'ra RefCell<NameResolution<'ra>>>>;
+type Resolutions<'ra> = CmRefCell<FxIndexMap<BindingKey, &'ra CmRefCell<NameResolution<'ra>>>>;
 
 /// One node in the tree of modules.
 ///
@@ -595,7 +595,7 @@ struct ModuleData<'ra> {
     /// Resolutions in modules from other crates are not populated until accessed.
     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>, // FIXME(parallel): Use an atomic in parallel import resolution
+    populate_on_access: CacheCell<bool>,
     /// Used to disambiguate underscore items (`const _: T = ...`) in the module.
     underscore_disambiguator: CmCell<u32>,
 
@@ -658,7 +658,7 @@ impl<'ra> ModuleData<'ra> {
             parent,
             kind,
             lazy_resolutions: Default::default(),
-            populate_on_access: Cell::new(is_foreign),
+            populate_on_access: CacheCell::new(is_foreign),
             underscore_disambiguator: CmCell::new(0),
             unexpanded_invocations: Default::default(),
             no_implicit_prelude,
@@ -1034,7 +1034,7 @@ struct ExternPreludeEntry<'ra> {
     /// `flag_binding` is `None`, or when `extern crate` introducing `item_binding` used renaming.
     item_binding: Option<(NameBinding<'ra>, /* introduced by item */ bool)>,
     /// Binding from an `--extern` flag, lazily populated on first use.
-    flag_binding: Option<Cell<(PendingBinding<'ra>, /* finalized */ bool)>>,
+    flag_binding: Option<CacheCell<(PendingBinding<'ra>, /* finalized */ bool)>>,
 }
 
 impl ExternPreludeEntry<'_> {
@@ -1045,7 +1045,7 @@ impl ExternPreludeEntry<'_> {
     fn flag() -> Self {
         ExternPreludeEntry {
             item_binding: None,
-            flag_binding: Some(Cell::new((PendingBinding::Pending, false))),
+            flag_binding: Some(CacheCell::new((PendingBinding::Pending, false))),
         }
     }
 }
@@ -1150,7 +1150,7 @@ pub struct Resolver<'ra, 'tcx> {
     /// Eagerly populated map of all local non-block modules.
     local_module_map: FxIndexMap<LocalDefId, Module<'ra>>,
     /// Lazily populated cache of modules loaded from external crates.
-    extern_module_map: RefCell<FxIndexMap<DefId, Module<'ra>>>,
+    extern_module_map: CacheRefCell<FxIndexMap<DefId, Module<'ra>>>,
     binding_parent_modules: FxHashMap<NameBinding<'ra>, Module<'ra>>,
 
     /// Maps glob imports to the names of items actually imported.
@@ -1186,7 +1186,7 @@ pub struct Resolver<'ra, 'tcx> {
     /// Eagerly populated map of all local macro definitions.
     local_macro_map: FxHashMap<LocalDefId, &'ra MacroData>,
     /// Lazily populated cache of macro definitions loaded from external crates.
-    extern_macro_map: RefCell<FxHashMap<DefId, &'ra MacroData>>,
+    extern_macro_map: CacheRefCell<FxHashMap<DefId, &'ra MacroData>>,
     dummy_ext_bang: Arc<SyntaxExtension>,
     dummy_ext_derive: Arc<SyntaxExtension>,
     non_macro_attr: &'ra MacroData,
@@ -1197,11 +1197,10 @@ pub struct Resolver<'ra, 'tcx> {
     unused_macro_rules: FxIndexMap<NodeId, DenseBitSet<usize>>,
     proc_macro_stubs: FxHashSet<LocalDefId>,
     /// Traces collected during macro resolution and validated when it's complete.
-    // FIXME: Remove interior mutability when speculative resolution produces these as outputs.
     single_segment_macro_resolutions:
-        RefCell<Vec<(Ident, MacroKind, ParentScope<'ra>, Option<NameBinding<'ra>>, Option<Span>)>>,
+        CmRefCell<Vec<(Ident, MacroKind, ParentScope<'ra>, Option<NameBinding<'ra>>, Option<Span>)>>,
     multi_segment_macro_resolutions:
-        RefCell<Vec<(Vec<Segment>, Span, MacroKind, ParentScope<'ra>, Option<Res>, Namespace)>>,
+        CmRefCell<Vec<(Vec<Segment>, Span, MacroKind, ParentScope<'ra>, Option<Res>, Namespace)>>,
     builtin_attrs: Vec<(Ident, ParentScope<'ra>)>,
     /// `derive(Copy)` marks items they are applied to so they are treated specially later.
     /// Derive macros cannot modify the item themselves and have to store the markers in the global
@@ -1301,7 +1300,7 @@ pub struct Resolver<'ra, 'tcx> {
 pub struct ResolverArenas<'ra> {
     modules: TypedArena<ModuleData<'ra>>,
     imports: TypedArena<ImportData<'ra>>,
-    name_resolutions: TypedArena<RefCell<NameResolution<'ra>>>,
+    name_resolutions: TypedArena<CmRefCell<NameResolution<'ra>>>,
     ast_paths: TypedArena<ast::Path>,
     macros: TypedArena<MacroData>,
     dropless: DroplessArena,
@@ -1363,11 +1362,11 @@ impl<'ra> ResolverArenas<'ra> {
     fn alloc_import(&'ra self, import: ImportData<'ra>) -> Import<'ra> {
         Interned::new_unchecked(self.imports.alloc(import))
     }
-    fn alloc_name_resolution(&'ra self) -> &'ra RefCell<NameResolution<'ra>> {
+    fn alloc_name_resolution(&'ra self) -> &'ra CmRefCell<NameResolution<'ra>> {
         self.name_resolutions.alloc(Default::default())
     }
     fn alloc_macro_rules_scope(&'ra self, scope: MacroRulesScope<'ra>) -> MacroRulesScopeRef<'ra> {
-        self.dropless.alloc(Cell::new(scope))
+        self.dropless.alloc(CacheCell::new(scope))
     }
     fn alloc_macro_rules_binding(
         &'ra self,
@@ -1978,7 +1977,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
 
     fn resolutions(&self, module: Module<'ra>) -> &'ra Resolutions<'ra> {
         if module.populate_on_access.get() {
-            // FIXME(batched): Will be fixed in batched import resolution.
             module.populate_on_access.set(false);
             self.build_reduced_graph_external(module);
         }
@@ -1997,9 +1995,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         &self,
         module: Module<'ra>,
         key: BindingKey,
-    ) -> &'ra RefCell<NameResolution<'ra>> {
+    ) -> &'ra CmRefCell<NameResolution<'ra>> {
         self.resolutions(module)
-            .borrow_mut()
+            .borrow_mut_unchecked()
             .entry(key)
             .or_insert_with(|| self.arenas.alloc_name_resolution())
     }
@@ -2514,6 +2512,13 @@ pub fn provide(providers: &mut Providers) {
 /// Prefer constructing it through [`Resolver::cm`] to ensure correctness.
 type CmResolver<'r, 'ra, 'tcx> = ref_mut::RefOrMut<'r, Resolver<'ra, 'tcx>>;
 
+// FIXME: These are cells for caches that can be populated even during speculative resolution,
+// and should be replaced with mutexes, atomics, or other synchronized data when migrating to
+// parallel name resolution.
+use std::cell::{Cell as CacheCell, RefCell as CacheRefCell};
+
+// FIXME: `*_unchecked` methods in the module below should be eliminated in the process
+// of migration to parallel name resolution.
 mod ref_mut {
     use std::cell::{BorrowMutError, Cell, Ref, RefCell, RefMut};
     use std::fmt;
@@ -2581,7 +2586,6 @@ mod ref_mut {
     }
 
     impl<T: Copy> Clone for CmCell<T> {
-        #[inline]
         fn clone(&self) -> CmCell<T> {
             CmCell::new(self.get())
         }
@@ -2624,13 +2628,11 @@ mod ref_mut {
             CmRefCell(RefCell::new(value))
         }
 
-        #[inline]
         #[track_caller]
         pub(crate) fn borrow_mut_unchecked(&self) -> RefMut<'_, T> {
             self.0.borrow_mut()
         }
 
-        #[inline]
         #[track_caller]
         pub(crate) fn borrow_mut<'ra, 'tcx>(&self, r: &Resolver<'ra, 'tcx>) -> RefMut<'_, T> {
             if r.assert_speculative {
@@ -2639,16 +2641,23 @@ mod ref_mut {
             self.borrow_mut_unchecked()
         }
 
-        #[inline]
         #[track_caller]
         pub(crate) fn try_borrow_mut_unchecked(&self) -> Result<RefMut<'_, T>, BorrowMutError> {
             self.0.try_borrow_mut()
         }
 
-        #[inline]
         #[track_caller]
         pub(crate) fn borrow(&self) -> Ref<'_, T> {
             self.0.borrow()
         }
     }
+
+    impl<T: Default> CmRefCell<T> {
+        pub(crate) fn take<'ra, 'tcx>(&self, r: &Resolver<'ra, 'tcx>) -> T {
+            if r.assert_speculative {
+                panic!("Not allowed to mutate a CmRefCell during speculative resolution");
+            }
+            self.0.take()
+        }
+    }
 }
diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs
@@ -1,7 +1,6 @@
 //! A bunch of methods and structures more or less related to resolving macros and
 //! interface provided by `Resolver` to macro expander.
 
-use std::cell::Cell;
 use std::mem;
 use std::sync::Arc;
 
@@ -40,9 +39,9 @@ use crate::errors::{
 };
 use crate::imports::Import;
 use crate::{
-    BindingKey, CmResolver, DeriveData, Determinacy, Finalize, InvocationParent, MacroData,
-    ModuleKind, ModuleOrUniformRoot, NameBinding, NameBindingKind, ParentScope, PathResult,
-    ResolutionError, Resolver, ScopeSet, Segment, Used,
+    BindingKey, CacheCell, CmResolver, DeriveData, Determinacy, Finalize, InvocationParent,
+    MacroData, ModuleKind, ModuleOrUniformRoot, NameBinding, NameBindingKind, ParentScope,
+    PathResult, ResolutionError, Resolver, ScopeSet, Segment, Used,
 };
 
 type Res = def::Res<NodeId>;
@@ -79,7 +78,7 @@ pub(crate) enum MacroRulesScope<'ra> {
 /// This helps to avoid uncontrollable growth of `macro_rules!` scope chains,
 /// which usually grow linearly with the number of macro invocations
 /// in a module (including derives) and hurt performance.
-pub(crate) type MacroRulesScopeRef<'ra> = &'ra Cell<MacroRulesScope<'ra>>;
+pub(crate) type MacroRulesScopeRef<'ra> = &'ra CacheCell<MacroRulesScope<'ra>>;
 
 /// Macro namespace is separated into two sub-namespaces, one for bang macros and
 /// one for attribute-like macros (attributes, derives).
@@ -775,8 +774,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             };
 
             if trace {
-                // FIXME: Should be an output of Speculative Resolution.
-                self.multi_segment_macro_resolutions.borrow_mut().push((
+                self.multi_segment_macro_resolutions.borrow_mut(&self).push((
                     path,
                     path_span,
                     kind,
@@ -803,8 +801,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             }
 
             if trace {
-                // FIXME: Should be an output of Speculative Resolution.
-                self.single_segment_macro_resolutions.borrow_mut().push((
+                self.single_segment_macro_resolutions.borrow_mut(&self).push((
                     path[0].ident,
                     kind,
                     *parent_scope,
@@ -870,8 +867,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             }
         };
 
-        // FIXME: Should be an output of Speculative Resolution.
-        let macro_resolutions = self.multi_segment_macro_resolutions.take();
+        let macro_resolutions = self.multi_segment_macro_resolutions.take(self);
         for (mut path, path_span, kind, parent_scope, initial_res, ns) in macro_resolutions {
             // FIXME: Path resolution will ICE if segment IDs present.
             for seg in &mut path {
@@ -948,8 +944,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             }
         }
 
-        // FIXME: Should be an output of Speculative Resolution.
-        let macro_resolutions = self.single_segment_macro_resolutions.take();
+        let macro_resolutions = self.single_segment_macro_resolutions.take(self);
         for (ident, kind, parent_scope, initial_binding, sugg_span) in macro_resolutions {
             match self.cm().resolve_ident_in_scope_set(
                 ident,
