Rollup merge of #146283 - LorrensP-2158466:resolve-cm-cell, r=petrochenkov

Resolve: (Ref)Cell wrappers to deny mutation during spec resolution.

Introduces wrappers around `Cell` and `RefCell` that only allow mutation when we are not in speculative resolution. This is preparatory work for rust-lang/rust#145108.

It would allow us to make `ImportData` and `ModuleData` sync and send safe.

r? ``@petrochenkov``

4 files changed, 135 insertions, 40 deletions

diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
index fa3c06059b3..c10b6ca7e71 100644
--- a/compiler/rustc_resolve/src/build_reduced_graph.rs
+++ b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -5,7 +5,6 @@
 //! unexpanded macros in the fragment are visited and registered.
 //! Imports are also considered items and placed into modules here, but not resolved yet.
 
-use std::cell::Cell;
 use std::sync::Arc;
 
 use rustc_ast::visit::{self, AssocCtxt, Visitor, WalkItemKind};
@@ -35,6 +34,7 @@ use crate::Namespace::{MacroNS, TypeNS, ValueNS};
 use crate::def_collector::collect_definitions;
 use crate::imports::{ImportData, ImportKind};
 use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
+use crate::ref_mut::CmCell;
 use crate::{
     BindingKey, ExternPreludeEntry, Finalize, MacroData, Module, ModuleKind, ModuleOrUniformRoot,
     NameBinding, ParentScope, PathResult, ResolutionError, Resolver, Segment, Used,
@@ -87,7 +87,8 @@ 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, || {
-            parent.underscore_disambiguator.update(|d| d + 1);
+            // FIXME(batched): Will be fixed in batched resolution.
+            parent.underscore_disambiguator.update_unchecked(|d| d + 1);
             parent.underscore_disambiguator.get()
         });
         if self
@@ -477,7 +478,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
             kind,
             parent_scope: self.parent_scope,
             module_path,
-            imported_module: Cell::new(None),
+            imported_module: CmCell::new(None),
             span,
             use_span: item.span,
             use_span_with_attributes: item.span_with_attributes(),
@@ -505,7 +506,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
                     });
                 }
             }
-            ImportKind::Glob { .. } => current_module.globs.borrow_mut().push(import),
+            ImportKind::Glob { .. } => current_module.globs.borrow_mut(self.r).push(import),
             _ => unreachable!(),
         }
     }
@@ -668,7 +669,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
             }
             ast::UseTreeKind::Glob => {
                 if !ast::attr::contains_name(&item.attrs, sym::prelude_import) {
-                    let kind = ImportKind::Glob { max_vis: Cell::new(None), id };
+                    let kind = ImportKind::Glob { max_vis: CmCell::new(None), id };
                     self.add_import(prefix, kind, use_tree.span, item, root_span, item.id, vis);
                 } else {
                     // Resolve the prelude import early.
@@ -971,7 +972,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
             kind: ImportKind::ExternCrate { source: orig_name, target: ident, id: item.id },
             root_id: item.id,
             parent_scope: self.parent_scope,
-            imported_module: Cell::new(module),
+            imported_module: CmCell::new(module),
             has_attributes: !item.attrs.is_empty(),
             use_span_with_attributes: item.span_with_attributes(),
             use_span: item.span,
@@ -1103,7 +1104,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
                 kind: ImportKind::MacroUse { warn_private },
                 root_id: item.id,
                 parent_scope: this.parent_scope,
-                imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
+                imported_module: CmCell::new(Some(ModuleOrUniformRoot::Module(module))),
                 use_span_with_attributes: item.span_with_attributes(),
                 has_attributes: !item.attrs.is_empty(),
                 use_span: item.span,
@@ -1196,7 +1197,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
     /// directly into its parent scope's module.
     fn visit_invoc_in_module(&mut self, id: NodeId) -> MacroRulesScopeRef<'ra> {
         let invoc_id = self.visit_invoc(id);
-        self.parent_scope.module.unexpanded_invocations.borrow_mut().insert(invoc_id);
+        self.parent_scope.module.unexpanded_invocations.borrow_mut(self.r).insert(invoc_id);
         self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Invocation(invoc_id))
     }
 
@@ -1274,7 +1275,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
                     kind: ImportKind::MacroExport,
                     root_id: item.id,
                     parent_scope: self.parent_scope,
-                    imported_module: Cell::new(None),
+                    imported_module: CmCell::new(None),
                     has_attributes: false,
                     use_span_with_attributes: span,
                     use_span: span,
diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
index 33c2c7436d1..ce90a1bcd31 100644
--- a/compiler/rustc_resolve/src/imports.rs
+++ b/compiler/rustc_resolve/src/imports.rs
@@ -1,6 +1,5 @@
 //! A bunch of methods and structures more or less related to resolving imports.
 
-use std::cell::Cell;
 use std::mem;
 
 use rustc_ast::NodeId;
@@ -32,6 +31,7 @@ use crate::errors::{
     CannotBeReexportedPrivateNS, CannotDetermineImportResolution, CannotGlobImportAllCrates,
     ConsiderAddingMacroExport, ConsiderMarkingAsPub, ConsiderMarkingAsPubCrate,
 };
+use crate::ref_mut::CmCell;
 use crate::{
     AmbiguityError, AmbiguityKind, BindingKey, CmResolver, Determinacy, Finalize, ImportSuggestion,
     Module, ModuleOrUniformRoot, NameBinding, NameBindingData, NameBindingKind, ParentScope,
@@ -68,7 +68,7 @@ pub(crate) enum ImportKind<'ra> {
         /// It will directly use `source` when the format is `use prefix::source`.
         target: Ident,
         /// Bindings introduced by the import.
-        bindings: PerNS<Cell<PendingBinding<'ra>>>,
+        bindings: PerNS<CmCell<PendingBinding<'ra>>>,
         /// `true` for `...::{self [as target]}` imports, `false` otherwise.
         type_ns_only: bool,
         /// Did this import result from a nested import? ie. `use foo::{bar, baz};`
@@ -89,7 +89,7 @@ pub(crate) enum ImportKind<'ra> {
     Glob {
         // The visibility of the greatest re-export.
         // n.b. `max_vis` is only used in `finalize_import` to check for re-export errors.
-        max_vis: Cell<Option<Visibility>>,
+        max_vis: CmCell<Option<Visibility>>,
         id: NodeId,
     },
     ExternCrate {
@@ -182,7 +182,7 @@ pub(crate) struct ImportData<'ra> {
     /// |`use ::foo`      | `ModuleOrUniformRoot::ExternPrelude`          | 2018+ editions |
     /// |`use ::foo`      | `ModuleOrUniformRoot::ModuleAndExternPrelude` | a special case in 2015 edition |
     /// |`use foo`        | `ModuleOrUniformRoot::CurrentScope`           | - |
-    pub imported_module: Cell<Option<ModuleOrUniformRoot<'ra>>>,
+    pub imported_module: CmCell<Option<ModuleOrUniformRoot<'ra>>>,
     pub vis: Visibility,
 
     /// Span of the visibility.
@@ -320,7 +320,8 @@ 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)))
         {
-            max_vis.set(Some(vis.expect_local()))
+            // FIXME(batched): Will be fixed in batched import resolution.
+            max_vis.set_unchecked(Some(vis.expect_local()))
         }
 
         self.arenas.alloc_name_binding(NameBindingData {
@@ -349,7 +350,8 @@ 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, || {
-            module.underscore_disambiguator.update(|d| d + 1);
+            // FIXME(batched): Will be fixed in batched resolution.
+            module.underscore_disambiguator.update_unchecked(|d| d + 1);
             module.underscore_disambiguator.get()
         });
         self.update_local_resolution(module, key, warn_ambiguity, |this, resolution| {
@@ -482,7 +484,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             }
         };
 
-        let Ok(glob_importers) = module.glob_importers.try_borrow_mut() else {
+        let Ok(glob_importers) = module.glob_importers.try_borrow_mut_unchecked() else {
             return t;
         };
 
@@ -862,7 +864,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             }
         };
 
-        import.imported_module.set(Some(module));
+        // 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)
@@ -937,7 +940,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         PendingBinding::Pending
                     }
                 };
-                bindings[ns].set(binding);
+                // FIXME(batched): Will be fixed in batched import resolution.
+                bindings[ns].set_unchecked(binding);
             }
         });
 
@@ -1508,7 +1512,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         }
 
         // Add to module's glob_importers
-        module.glob_importers.borrow_mut().push(import);
+        module.glob_importers.borrow_mut_unchecked().push(import);
 
         // Ensure that `resolutions` isn't borrowed during `try_define`,
         // since it might get updated via a glob cycle.
@@ -1550,7 +1554,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
     // reporting conflicts, and reporting unresolved imports.
     fn finalize_resolutions_in(&mut self, module: Module<'ra>) {
         // Since import resolution is finished, globs will not define any more names.
-        *module.globs.borrow_mut() = Vec::new();
+        *module.globs.borrow_mut(self) = Vec::new();
 
         let Some(def_id) = module.opt_def_id() else { return };
 
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
index 8b185ce7ef2..8959068b2a6 100644
--- a/compiler/rustc_resolve/src/lib.rs
+++ b/compiler/rustc_resolve/src/lib.rs
@@ -19,6 +19,7 @@
 #![feature(default_field_values)]
 #![feature(if_let_guard)]
 #![feature(iter_intersperse)]
+#![feature(ptr_as_ref_unchecked)]
 #![feature(rustc_attrs)]
 #![feature(rustdoc_internals)]
 #![recursion_limit = "256"]
@@ -26,7 +27,7 @@
 
 use std::cell::{Cell, Ref, RefCell};
 use std::collections::BTreeSet;
-use std::fmt;
+use std::fmt::{self};
 use std::sync::Arc;
 
 use diagnostics::{ImportSuggestion, LabelSuggestion, Suggestion};
@@ -95,6 +96,8 @@ pub mod rustdoc;
 
 pub use macros::registered_tools_ast;
 
+use crate::ref_mut::{CmCell, CmRefCell};
+
 rustc_fluent_macro::fluent_messages! { "../messages.ftl" }
 
 #[derive(Debug)]
@@ -592,22 +595,22 @@ 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>,
+    populate_on_access: Cell<bool>, // FIXME(parallel): Use an atomic in parallel import resolution
     /// Used to disambiguate underscore items (`const _: T = ...`) in the module.
-    underscore_disambiguator: Cell<u32>,
+    underscore_disambiguator: CmCell<u32>,
 
     /// Macro invocations that can expand into items in this module.
-    unexpanded_invocations: RefCell<FxHashSet<LocalExpnId>>,
+    unexpanded_invocations: CmRefCell<FxHashSet<LocalExpnId>>,
 
     /// Whether `#[no_implicit_prelude]` is active.
     no_implicit_prelude: bool,
 
-    glob_importers: RefCell<Vec<Import<'ra>>>,
-    globs: RefCell<Vec<Import<'ra>>>,
+    glob_importers: CmRefCell<Vec<Import<'ra>>>,
+    globs: CmRefCell<Vec<Import<'ra>>>,
 
     /// Used to memoize the traits in this module for faster searches through all traits in scope.
     traits:
-        RefCell<Option<Box<[(Macros20NormalizedIdent, NameBinding<'ra>, Option<Module<'ra>>)]>>>,
+        CmRefCell<Option<Box<[(Macros20NormalizedIdent, NameBinding<'ra>, Option<Module<'ra>>)]>>>,
 
     /// Span of the module itself. Used for error reporting.
     span: Span,
@@ -656,12 +659,12 @@ impl<'ra> ModuleData<'ra> {
             kind,
             lazy_resolutions: Default::default(),
             populate_on_access: Cell::new(is_foreign),
-            underscore_disambiguator: Cell::new(0),
+            underscore_disambiguator: CmCell::new(0),
             unexpanded_invocations: Default::default(),
             no_implicit_prelude,
-            glob_importers: RefCell::new(Vec::new()),
-            globs: RefCell::new(Vec::new()),
-            traits: RefCell::new(None),
+            glob_importers: CmRefCell::new(Vec::new()),
+            globs: CmRefCell::new(Vec::new()),
+            traits: CmRefCell::new(None),
             span,
             expansion,
             self_binding,
@@ -696,7 +699,7 @@ impl<'ra> Module<'ra> {
 
     /// This modifies `self` in place. The traits will be stored in `self.traits`.
     fn ensure_traits<'tcx>(self, resolver: &impl AsRef<Resolver<'ra, 'tcx>>) {
-        let mut traits = self.traits.borrow_mut();
+        let mut traits = self.traits.borrow_mut(resolver.as_ref());
         if traits.is_none() {
             let mut collected_traits = Vec::new();
             self.for_each_child(resolver, |r, name, ns, binding| {
@@ -1974,6 +1977,7 @@ 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);
         }
@@ -2504,9 +2508,20 @@ pub fn provide(providers: &mut Providers) {
     providers.registered_tools = macros::registered_tools;
 }
 
+/// A wrapper around `&mut Resolver` that may be mutable or immutable, depending on a conditions.
+///
+/// `Cm` stands for "conditionally mutable".
+///
+/// Prefer constructing it through [`Resolver::cm`] to ensure correctness.
+type CmResolver<'r, 'ra, 'tcx> = ref_mut::RefOrMut<'r, Resolver<'ra, 'tcx>>;
+
 mod ref_mut {
+    use std::cell::{BorrowMutError, Cell, Ref, RefCell, RefMut};
+    use std::fmt;
     use std::ops::Deref;
 
+    use crate::Resolver;
+
     /// A wrapper around a mutable reference that conditionally allows mutable access.
     pub(crate) struct RefOrMut<'a, T> {
         p: &'a mut T,
@@ -2555,11 +2570,86 @@ mod ref_mut {
             self.p
         }
     }
-}
 
-/// A wrapper around `&mut Resolver` that may be mutable or immutable, depending on a conditions.
-///
-/// `Cm` stands for "conditionally mutable".
-///
-/// Prefer constructing it through [`Resolver::cm`] to ensure correctness.
-type CmResolver<'r, 'ra, 'tcx> = ref_mut::RefOrMut<'r, Resolver<'ra, 'tcx>>;
+    /// A wrapper around a [`Cell`] that only allows mutation based on a condition in the resolver.
+    #[derive(Default)]
+    pub(crate) struct CmCell<T>(Cell<T>);
+
+    impl<T: Copy + fmt::Debug> fmt::Debug for CmCell<T> {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            f.debug_tuple("CmCell").field(&self.get()).finish()
+        }
+    }
+
+    impl<T: Copy> Clone for CmCell<T> {
+        #[inline]
+        fn clone(&self) -> CmCell<T> {
+            CmCell::new(self.get())
+        }
+    }
+
+    impl<T: Copy> CmCell<T> {
+        pub(crate) const fn get(&self) -> T {
+            self.0.get()
+        }
+
+        pub(crate) fn update_unchecked(&self, f: impl FnOnce(T) -> T)
+        where
+            T: Copy,
+        {
+            let old = self.get();
+            self.set_unchecked(f(old));
+        }
+    }
+
+    impl<T> CmCell<T> {
+        pub(crate) const fn new(value: T) -> CmCell<T> {
+            CmCell(Cell::new(value))
+        }
+
+        pub(crate) fn set_unchecked(&self, val: T) {
+            self.0.set(val);
+        }
+
+        pub(crate) fn into_inner(self) -> T {
+            self.0.into_inner()
+        }
+    }
+
+    /// A wrapper around a [`RefCell`] that only allows mutable borrows based on a condition in the resolver.
+    #[derive(Default)]
+    pub(crate) struct CmRefCell<T>(RefCell<T>);
+
+    impl<T> CmRefCell<T> {
+        pub(crate) const fn new(value: T) -> CmRefCell<T> {
+            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 {
+                panic!("Not allowed to mutably borrow a CmRefCell during speculative resolution");
+            }
+            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()
+        }
+    }
+}
diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs
index d3e98ef839b..c50dfd41b51 100644
--- a/compiler/rustc_resolve/src/macros.rs
+++ b/compiler/rustc_resolve/src/macros.rs
@@ -189,7 +189,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
         let output_macro_rules_scope = self.build_reduced_graph(fragment, parent_scope);
         self.output_macro_rules_scopes.insert(expansion, output_macro_rules_scope);
 
-        parent_scope.module.unexpanded_invocations.borrow_mut().remove(&expansion);
+        parent_scope.module.unexpanded_invocations.borrow_mut(self).remove(&expansion);
         if let Some(unexpanded_invocations) =
             self.impl_unexpanded_invocations.get_mut(&self.invocation_parent(expansion))
         {