user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Rollup merge of #147539 - petrochenkov:cmresolve, r=eholk

Browse source 
resolve: Use primitives for conditional mutability more consistently

No bare `(Ref)Cell`s remain in `rustc_resolve`, only `Cm(Ref)Cell`s checking that nothing is modified during speculative resolution, and `Cache(Ref)Cell` aliases for cells that need to be migrated to mutexes/atomics.

cc `@LorrensP-2158466`
This commit is contained in:
Matthias Krüger 2025-10-15 07:09:55 +02:00 • committed by GitHub
commit 33b1e92d4f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 71 additions and 77 deletions
Download patch file Download diff file Expand all files Collapse all files

5
compiler/rustc_resolve/src/build_reduced_graph.rs
View file

@ -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);
}

2
compiler/rustc_resolve/src/check_unused.rs
View file

@ -507,7 +507,7 @@ impl Resolver<'_, '_> {
let unused_imports = visitor.unused_imports;
let mut check_redundant_imports = FxIndexSet::default();
for module in self.arenas.local_modules().iter() {
for module in &self.local_modules {
for (_key, resolution) in self.resolutions(*module).borrow().iter() {
if let Some(binding) = resolution.borrow().best_binding()
&& let NameBindingKind::Import { import, .. } = binding.kind

2
compiler/rustc_resolve/src/ident.rs
View file

@ -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

34
compiler/rustc_resolve/src/imports.rs
View file

@ -8,7 +8,7 @@ use rustc_data_structures::intern::Interned;
use rustc_errors::codes::*;
use rustc_errors::{Applicability, MultiSpan, pluralize, struct_span_code_err};
use rustc_hir::def::{self, DefKind, PartialRes};
use rustc_hir::def_id::DefId;
use rustc_hir::def_id::{DefId, LocalDefIdMap};
use rustc_middle::metadata::{ModChild, Reexport};
use rustc_middle::span_bug;
use rustc_middle::ty::Visibility;
@ -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,14 +565,16 @@ 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) {
for module in self.arenas.local_modules().iter() {
self.finalize_resolutions_in(*module);
let mut module_children = Default::default();
for module in &self.local_modules {
self.finalize_resolutions_in(*module, &mut module_children);
}
self.module_children = module_children;
let mut seen_spans = FxHashSet::default();
let mut errors = vec![];
@ -651,7 +651,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}
pub(crate) fn lint_reexports(&mut self, exported_ambiguities: FxHashSet<NameBinding<'ra>>) {
for module in self.arenas.local_modules().iter() {
for module in &self.local_modules {
for (key, resolution) in self.resolutions(*module).borrow().iter() {
let resolution = resolution.borrow();
let Some(binding) = resolution.best_binding() else { continue };
@ -860,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;
}
@ -904,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,
@ -918,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,
@ -936,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);
}
});
@ -1548,7 +1540,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// Miscellaneous post-processing, including recording re-exports,
// reporting conflicts, and reporting unresolved imports.
fn finalize_resolutions_in(&mut self, module: Module<'ra>) {
fn finalize_resolutions_in(
&self,
module: Module<'ra>,
module_children: &mut LocalDefIdMap<Vec<ModChild>>,
) {
// Since import resolution is finished, globs will not define any more names.
*module.globs.borrow_mut(self) = Vec::new();
@ -1573,7 +1569,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
if !children.is_empty() {
// Should be fine because this code is only called for local modules.
self.module_children.insert(def_id.expect_local(), children);
module_children.insert(def_id.expect_local(), children);
}
}
}

84
compiler/rustc_resolve/src/lib.rs
View file

@ -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))),
}
}
}
@ -1145,10 +1145,12 @@ pub struct Resolver<'ra, 'tcx> {
/// some AST passes can generate identifiers that only resolve to local or
/// lang items.
empty_module: Module<'ra>,
/// All local modules, including blocks.
local_modules: Vec<Module<'ra>>,
/// 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.
@ -1184,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,
@ -1195,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
@ -1298,9 +1299,8 @@ pub struct Resolver<'ra, 'tcx> {
#[derive(Default)]
pub struct ResolverArenas<'ra> {
modules: TypedArena<ModuleData<'ra>>,
local_modules: RefCell<Vec<Module<'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,
@ -1341,28 +1341,20 @@ impl<'ra> ResolverArenas<'ra> {
span: Span,
no_implicit_prelude: bool,
) -> Module<'ra> {
let (def_id, self_binding) = match kind {
ModuleKind::Def(def_kind, def_id, _) => (
Some(def_id),
Some(self.new_pub_res_binding(Res::Def(def_kind, def_id), span, LocalExpnId::ROOT)),
),
ModuleKind::Block => (None, None),
let self_binding = match kind {
ModuleKind::Def(def_kind, def_id, _) => {
Some(self.new_pub_res_binding(Res::Def(def_kind, def_id), span, LocalExpnId::ROOT))
}
ModuleKind::Block => None,
};
let module = Module(Interned::new_unchecked(self.modules.alloc(ModuleData::new(
Module(Interned::new_unchecked(self.modules.alloc(ModuleData::new(
parent,
kind,
expn_id,
span,
no_implicit_prelude,
self_binding,
))));
if def_id.is_none_or(|def_id| def_id.is_local()) {
self.local_modules.borrow_mut().push(module);
}
module
}
fn local_modules(&'ra self) -> std::cell::Ref<'ra, Vec<Module<'ra>>> {
self.local_modules.borrow()
))))
}
fn alloc_name_binding(&'ra self, name_binding: NameBindingData<'ra>) -> NameBinding<'ra> {
Interned::new_unchecked(self.dropless.alloc(name_binding))
@ -1370,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,
@ -1505,7 +1497,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
arenas: &'ra ResolverArenas<'ra>,
) -> Resolver<'ra, 'tcx> {
let root_def_id = CRATE_DEF_ID.to_def_id();
let mut local_module_map = FxIndexMap::default();
let graph_root = arenas.new_module(
None,
ModuleKind::Def(DefKind::Mod, root_def_id, None),
@ -1513,7 +1504,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
crate_span,
attr::contains_name(attrs, sym::no_implicit_prelude),
);
local_module_map.insert(CRATE_DEF_ID, graph_root);
let local_modules = vec![graph_root];
let local_module_map = FxIndexMap::from_iter([(CRATE_DEF_ID, graph_root)]);
let empty_module = arenas.new_module(
None,
ModuleKind::Def(DefKind::Mod, root_def_id, None),
@ -1591,6 +1583,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
module_children: Default::default(),
trait_map: NodeMap::default(),
empty_module,
local_modules,
local_module_map,
extern_module_map: Default::default(),
block_map: Default::default(),
@ -1694,6 +1687,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
no_implicit_prelude: bool,
) -> Module<'ra> {
let module = self.arenas.new_module(parent, kind, expn_id, span, no_implicit_prelude);
self.local_modules.push(module);
if let Some(def_id) = module.opt_def_id() {
self.local_module_map.insert(def_id.expect_local(), module);
}
@ -1983,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);
}
@ -2002,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())
}
@ -2533,6 +2526,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;
@ -2600,7 +2600,6 @@ mod ref_mut {
}
impl<T: Copy> Clone for CmCell<T> {
#[inline]
fn clone(&self) -> CmCell<T> {
CmCell::new(self.get())
}
@ -2643,13 +2642,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 {
@ -2658,16 +2655,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()
}
}
}

21
compiler/rustc_resolve/src/macros.rs
View file

@ -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,