address review

Author: LorrensP-2158466

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -34,10 +34,11 @@ 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, CmCell, ExternPreludeEntry, Finalize, MacroData, Module, ModuleKind,
-    ModuleOrUniformRoot, NameBinding, ParentScope, PathResult, ResolutionError, Resolver, Segment,
-    Used, VisResolutionError, errors,
+    BindingKey, ExternPreludeEntry, Finalize, MacroData, Module, ModuleKind, ModuleOrUniformRoot,
+    NameBinding, ParentScope, PathResult, ResolutionError, Resolver, Segment, Used,
+    VisResolutionError, errors,
 };
 
 type Res = def::Res<NodeId>;
@@ -505,7 +506,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
                     });
                 }
             }
-            ImportKind::Glob { .. } => current_module.globs.borrow_mut_unchecked().push(import),
+            ImportKind::Glob { .. } => current_module.globs.borrow_mut(self.r).push(import),
             _ => unreachable!(),
         }
     }
@@ -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_unchecked().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))
     }
 

compiler/rustc_resolve/src/imports.rs

@@ -31,11 +31,12 @@ use crate::errors::{
     CannotBeReexportedPrivateNS, CannotDetermineImportResolution, CannotGlobImportAllCrates,
     ConsiderAddingMacroExport, ConsiderMarkingAsPub, ConsiderMarkingAsPubCrate,
 };
+use crate::ref_mut::CmCell;
 use crate::{
-    AmbiguityError, AmbiguityKind, BindingKey, CmCell, CmResolver, Determinacy, Finalize,
-    ImportSuggestion, Module, ModuleOrUniformRoot, NameBinding, NameBindingData, NameBindingKind,
-    ParentScope, PathResult, PerNS, ResolutionError, Resolver, ScopeSet, Segment, Used,
-    module_to_string, names_to_string,
+    AmbiguityError, AmbiguityKind, BindingKey, CmResolver, Determinacy, Finalize, ImportSuggestion,
+    Module, ModuleOrUniformRoot, NameBinding, NameBindingData, NameBindingKind, ParentScope,
+    PathResult, PerNS, ResolutionError, Resolver, ScopeSet, Segment, Used, module_to_string,
+    names_to_string,
 };
 
 type Res = def::Res<NodeId>;
@@ -1553,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_unchecked() = Vec::new();
+        *module.globs.borrow_mut(self) = Vec::new();
 
         let Some(def_id) = module.opt_def_id() else { return };
 

compiler/rustc_resolve/src/lib.rs

@@ -25,9 +25,9 @@
 #![recursion_limit = "256"]
 // tidy-alphabetical-end
 
-use std::cell::{BorrowMutError, Cell, Ref, RefCell, RefMut};
+use std::cell::{Cell, Ref, RefCell};
 use std::collections::BTreeSet;
-use std::fmt::{self, Formatter};
+use std::fmt::{self};
 use std::sync::Arc;
 
 use diagnostics::{ImportSuggestion, LabelSuggestion, Suggestion};
@@ -96,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)]
@@ -593,9 +595,9 @@ 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: CmCell<bool>, // FIXME(batched): Use an atomic in batched import resolution?
+    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: CmCell<u32>, // FIXME(batched): Use an atomic in batched import resolution?
+    underscore_disambiguator: CmCell<u32>,
 
     /// Macro invocations that can expand into items in this module.
     unexpanded_invocations: CmRefCell<FxHashSet<LocalExpnId>>,
@@ -656,7 +658,7 @@ impl<'ra> ModuleData<'ra> {
             parent,
             kind,
             lazy_resolutions: Default::default(),
-            populate_on_access: CmCell::new(is_foreign),
+            populate_on_access: Cell::new(is_foreign),
             underscore_disambiguator: CmCell::new(0),
             unexpanded_invocations: Default::default(),
             no_implicit_prelude,
@@ -697,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_unchecked();
+        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| {
@@ -1976,7 +1978,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_unchecked(false);
+            module.populate_on_access.set(false);
             self.build_reduced_graph_external(module);
         }
         &module.0.0.lazy_resolutions
@@ -2504,9 +2506,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,117 +2568,84 @@ 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>>;
-
-#[derive(Default)]
-struct CmCell<T> {
-    value: Cell<T>,
-}
-
-impl<T: Copy + fmt::Debug> fmt::Debug for CmCell<T> {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        f.debug_struct("CmCell").field("value", &self.get()).finish()
-    }
-}
+    #[derive(Default)]
+    pub(crate) struct CmCell<T>(Cell<T>);
 
-impl<T: Copy> Clone for CmCell<T> {
-    #[inline]
-    fn clone(&self) -> CmCell<T> {
-        CmCell::new(self.get())
+    impl<T: Copy + fmt::Debug> fmt::Debug for CmCell<T> {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            f.debug_struct("CmCell").field("value", &self.get()).finish()
+        }
     }
-}
 
-impl<T: Copy> CmCell<T> {
-    const fn get(&self) -> T {
-        self.value.get()
+    impl<T: Copy> Clone for CmCell<T> {
+        #[inline]
+        fn clone(&self) -> CmCell<T> {
+            CmCell::new(self.get())
+        }
     }
 
-    fn update_unchecked(&self, f: impl FnOnce(T) -> T)
-    where
-        T: Copy,
-    {
-        let old = self.get();
-        self.set_unchecked(f(old));
-    }
-}
+    impl<T: Copy> CmCell<T> {
+        pub(crate) const fn get(&self) -> T {
+            self.0.get()
+        }
 
-impl<T> CmCell<T> {
-    const fn new(value: T) -> CmCell<T> {
-        CmCell { value: Cell::new(value) }
+        pub(crate) fn update_unchecked(&self, f: impl FnOnce(T) -> T)
+        where
+            T: Copy,
+        {
+            let old = self.get();
+            self.set_unchecked(f(old));
+        }
     }
 
-    #[track_caller]
-    #[allow(dead_code)]
-    fn set<'ra, 'tcx>(&self, val: T, r: &Resolver<'ra, 'tcx>) {
-        if r.assert_speculative {
-            panic!("Not allowed to mutate CmCell during speculative resolution");
+    impl<T> CmCell<T> {
+        pub(crate) const fn new(value: T) -> CmCell<T> {
+            CmCell(Cell::new(value))
         }
-        self.value.set(val);
-    }
 
-    fn set_unchecked(&self, val: T) {
-        self.value.set(val);
-    }
+        pub(crate) fn set_unchecked(&self, val: T) {
+            self.0.set(val);
+        }
 
-    fn into_inner(self) -> T {
-        self.value.into_inner()
+        pub(crate) fn into_inner(self) -> T {
+            self.0.into_inner()
+        }
     }
-}
-#[derive(Default)]
-struct CmRefCell<T> {
-    ref_cell: RefCell<T>,
-}
 
-impl<T> CmRefCell<T> {
-    const fn new(value: T) -> CmRefCell<T> {
-        CmRefCell { ref_cell: RefCell::new(value) }
-    }
+    #[derive(Default)]
+    pub(crate) struct CmRefCell<T>(RefCell<T>);
 
-    #[inline]
-    #[track_caller]
-    #[allow(dead_code)]
-    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");
+    impl<T> CmRefCell<T> {
+        pub(crate) const fn new(value: T) -> CmRefCell<T> {
+            CmRefCell(RefCell::new(value))
         }
-        self.ref_cell.borrow_mut()
-    }
 
-    #[inline]
-    #[track_caller]
-    fn borrow_mut_unchecked(&self) -> RefMut<'_, T> {
-        self.ref_cell.borrow_mut()
-    }
+        #[inline]
+        #[track_caller]
+        pub(crate) fn borrow_mut_unchecked(&self) -> RefMut<'_, T> {
+            self.0.borrow_mut()
+        }
 
-    #[inline]
-    #[track_caller]
-    fn try_borrow_mut_unchecked(&self) -> Result<RefMut<'_, T>, BorrowMutError> {
-        self.ref_cell.try_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]
-    #[allow(dead_code)]
-    fn try_borrow_mut<'ra, 'tcx>(
-        &self,
-        r: Resolver<'ra, 'tcx>,
-    ) -> Result<RefMut<'_, T>, BorrowMutError> {
-        if r.assert_speculative {
-            panic!("Not allowed to mutably borrow a CmRefCell during speculative resolution");
+        #[inline]
+        #[track_caller]
+        pub(crate) fn try_borrow_mut_unchecked(&self) -> Result<RefMut<'_, T>, BorrowMutError> {
+            self.0.try_borrow_mut()
         }
-        self.ref_cell.try_borrow_mut()
-    }
 
-    #[inline]
-    #[track_caller]
-    fn borrow(&self) -> Ref<'_, T> {
-        self.ref_cell.borrow()
+        #[inline]
+        #[track_caller]
+        pub(crate) fn borrow(&self) -> Ref<'_, T> {
+            self.0.borrow()
+        }
     }
 }

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_unchecked().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))
         {