diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index 41573749abbe7..dcbda2f96323e 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -559,7 +559,7 @@ impl Resolver<'_, '_> { let mut check_redundant_imports = FxIndexSet::default(); for module in &self.local_modules { for (_key, resolution) in self.resolutions(module.to_module()).iter() { - if let Some(decl) = resolution.borrow().best_decl() + if let Some(decl) = resolution.borrow(self).best_decl() && let DeclKind::Import { import, .. } = decl.kind && let ImportKind::Single { id, .. } = import.kind { diff --git a/compiler/rustc_resolve/src/diagnostics/impls.rs b/compiler/rustc_resolve/src/diagnostics/impls.rs index cc2c72ad59906..4e451665398e9 100644 --- a/compiler/rustc_resolve/src/diagnostics/impls.rs +++ b/compiler/rustc_resolve/src/diagnostics/impls.rs @@ -1873,7 +1873,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { self.resolutions(parent_scope.module).iter().any(|(key, name_resolution)| { if key.ns == TypeNS && key.ident == *ident - && let Some(decl) = name_resolution.borrow().best_decl() + && let Some(decl) = name_resolution.borrow(self).best_decl() { match decl.res() { // No disambiguation needed if the identically named item we @@ -3603,7 +3603,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let mut res = false; let m = r.expect_module(parent_module); if m.is_local() { - for importer in m.glob_importers.borrow().iter() { + for importer in m.glob_importers.borrow(r).iter() { if let Some(next_parent_module) = importer.parent_scope.module.opt_def_id() { if next_parent_module == module diff --git a/compiler/rustc_resolve/src/effective_visibilities.rs b/compiler/rustc_resolve/src/effective_visibilities.rs index ff976b080d40d..840a8a8682538 100644 --- a/compiler/rustc_resolve/src/effective_visibilities.rs +++ b/compiler/rustc_resolve/src/effective_visibilities.rs @@ -126,7 +126,7 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> { fn set_bindings_effective_visibilities(&mut self, module_id: LocalDefId) { let module = self.r.expect_module(module_id.to_def_id()); for (_, name_resolution) in self.r.resolutions(module).iter() { - let Some(decl) = name_resolution.borrow().best_decl() else { + let Some(decl) = name_resolution.borrow(self.r).best_decl() else { continue; }; self.update_decl_chain(decl, ParentId::Def(module_id)); @@ -310,7 +310,7 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> { if self.macro_reachable.insert((module_def_id, defining_mod)) { let module = self.r.expect_module(module_def_id.to_def_id()); for (_, name_resolution) in self.r.resolutions(module).iter() { - let Some(decl) = name_resolution.borrow().best_decl() else { + let Some(decl) = name_resolution.borrow(self.r).best_decl() else { continue; }; diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 3f34af1d01d83..42fc5964f5aa1 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -714,7 +714,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } Scope::MacroUsePrelude => match self.macro_use_prelude.get(&ident.name).cloned() { Some(decl) => Ok(decl), - None => Err(Determinacy::determined(!self.graph_root.has_unexpanded_invocations())), + None => { + Err(Determinacy::determined(!self.graph_root.has_unexpanded_invocations(&self))) + } }, Scope::BuiltinAttrs => match self.builtin_attr_decls.get(&ident.name) { Some(decl) => Ok(*decl), @@ -727,9 +729,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { finalize.is_some(), ) { Some(decl) => Ok(decl), - None => { - Err(Determinacy::determined(!self.graph_root.has_unexpanded_invocations())) - } + None => Err(Determinacy::determined( + !self.graph_root.has_unexpanded_invocations(&self), + )), } } Scope::ExternPreludeFlags => { @@ -1158,7 +1160,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { if let Some(finalize) = finalize { // finalize implies that the module is fully expanded - assert!(!module.has_unexpanded_invocations()); + assert!(!module.has_unexpanded_invocations(&self)); return self.get_mut().finalize_module_binding( ident, orig_ident_span, @@ -1195,7 +1197,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } // Check if one of unexpanded macros can still define the name. - if module.has_unexpanded_invocations() { + if module.has_unexpanded_invocations(&self) { return Err(ControlFlow::Continue(Undetermined)); } @@ -1224,7 +1226,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { if let Some(finalize) = finalize { // finalize implies that the module is fully expanded - assert!(!module.has_unexpanded_invocations()); + assert!(!module.has_unexpanded_invocations(&self)); return self.get_mut().finalize_module_binding( ident, orig_ident_span, @@ -1268,7 +1270,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // and prohibit access to macro-expanded `macro_export` macros instead (unless restricted // shadowing is enabled, see `macro_expanded_macro_export_errors`). if let Some(binding) = binding { - return if binding.determined() || ns == MacroNS || shadowing == Shadowing::Restricted { + return if binding.determined(&self) + || ns == MacroNS + || shadowing == Shadowing::Restricted + { let accessible = self.is_accessible_from(binding.vis(), parent_scope.module); if accessible { Ok(binding) } else { Err(ControlFlow::Break(Determined)) } } else { @@ -1283,13 +1288,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // scopes we return `Undetermined` with `ControlFlow::Continue`. // Check if one of unexpanded macros can still define the name, // if it can then our "no resolution" result is not determined and can be invalidated. - if module.has_unexpanded_invocations() { + if module.has_unexpanded_invocations(&self) { return Err(ControlFlow::Continue(Undetermined)); } // Check if one of glob imports can still define the name, // if it can then our "no resolution" result is not determined and can be invalidated. - for glob_import in module.globs.borrow().iter() { + for glob_import in module.globs.borrow(&self).iter() { if ignore_import == Some(*glob_import) { continue; } diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 6e2ea9abf2de8..499f9ea297362 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -781,14 +781,22 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let mut imports_to_resolve = mem::take(&mut self.indeterminate_imports); - self.assert_speculative = true; + // SAFETY: This is a "top-level" function used by the macro expansion code, unless some + // weird thing is done, all `tracked` borrows done in the previous call of + // `resolve_imports` are dropped when that call ended. + unsafe { self.speculative_flag.set(true) }; rustc_data_structures::sync::par_for_each_slice( &mut imports_to_resolve, |(import, resolution, indeterminate_count)| { (*resolution, *indeterminate_count) = self.resolve_import(*import); }, ); - self.assert_speculative = false; + // SAFETY: All `untracked` borrows are dropped after the `par_for_each_slice` call, + // as they cannot escape since they are tied to the `CmRefCell` they borrowed from. + // + // Note: Some `CmRefCell`s are arena allocated and thus have the `'ra` lifetime, + // allowing these borrows to escape, but that does not and should not happen. + unsafe { self.speculative_flag.set(false) }; self.write_import_resolutions(&imports_to_resolve); @@ -1003,7 +1011,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { pub(crate) fn lint_reexports(&mut self, exported_ambiguities: FxHashSet>) { for module in &self.local_modules { for (key, resolution) in self.resolutions(module.to_module()).iter() { - let resolution = resolution.borrow(); + let resolution = resolution.borrow(self); let Some(binding) = resolution.best_decl() else { continue }; // Report "cannot reexport" errors for exotic cases involving macros 2.0 @@ -1490,7 +1498,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { return None; } // `use _` is never valid - let resolution = resolution.borrow(); + let resolution = resolution.borrow(self); if let Some(name_binding) = resolution.best_decl() { match name_binding.kind { DeclKind::Import { source_decl, .. } => { @@ -1800,7 +1808,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { .resolutions(module) .iter() .filter_map(|(key, resolution)| { - let res = resolution.borrow(); + let res = resolution.borrow(self); let decl = res.determined_decl()?; let mut key = *key; let scope = match key.ident.ctxt.update_unchecked(|ctxt| { diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 6350f79ed007f..b126272583692 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -194,7 +194,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { if key.ident.name != assoc_name { return None; } - let resolution = resolution.borrow(); + let resolution = resolution.borrow(self.r); let binding = resolution.best_decl()?; match binding.res() { Res::Def(DefKind::AssocTy, def_id) => Some(def_id), @@ -1165,7 +1165,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { let find_doc_alias_name = |r: &mut Resolver<'ra, '_>, m: Module<'ra>, item_name: Symbol| { for resolution in r.resolutions(m).values() { let Some(did) = - resolution.borrow().best_decl().and_then(|binding| binding.res().opt_def_id()) + resolution.borrow(r).best_decl().and_then(|binding| binding.res().opt_def_id()) else { continue; }; @@ -1905,7 +1905,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { .resolutions(module) .iter() .filter_map(|(key, resolution)| { - let resolution = resolution.borrow(); + let resolution = resolution.borrow(self.r); resolution.best_decl().map(|binding| binding.res()).and_then(|res| { if filter_fn(res) { Some((key.ident.name, resolution.orig_ident_span, res)) @@ -2766,7 +2766,9 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { .r .resolutions(*module) .iter() - .filter_map(|(key, res)| res.borrow().best_decl().map(|binding| (key, binding.res()))) + .filter_map(|(key, res)| { + res.borrow(self.r).best_decl().map(|binding| (key, binding.res())) + }) .filter(|(_, res)| match (kind, res) { (AssocItemKind::Const(..), Res::Def(DefKind::AssocConst { .. }, _)) => true, (AssocItemKind::Fn(_), Res::Def(DefKind::AssocFn, _)) => true, diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index a3c804e56ee22..b7e57ad8ec37e 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -21,7 +21,7 @@ #![recursion_limit = "256"] // tidy-alphabetical-end -use std::cell::{Ref, RefMut}; +use std::cell::RefMut; use std::collections::BTreeSet; use std::ops::ControlFlow; use std::sync::{Arc, OnceLock}; @@ -81,6 +81,7 @@ use crate::diagnostics::impls::{ ImportSuggestion, LabelSuggestion, OnUnknownData, StructCtor, Suggestion, }; use crate::imports::{ImportResolution, NameResolutionRef}; +use crate::ref_mut::speculative::SpeculativeFlag; use crate::ref_mut::{CmCell, CmRef, CmRefCell}; mod build_reduced_graph; @@ -767,8 +768,8 @@ impl<'ra> ModuleData<'ra> { self.kind.is_local() } - fn has_unexpanded_invocations(&self) -> bool { - !self.unexpanded_invocations.borrow().is_empty() + fn has_unexpanded_invocations<'tcx>(&self, r: &Resolver<'ra, 'tcx>) -> bool { + !self.unexpanded_invocations.borrow(r).is_empty() } fn res(&self) -> Option { @@ -793,7 +794,7 @@ impl<'ra> Module<'ra> { mut f: impl FnMut(&R, IdentKey, Span, Namespace, Decl<'ra>), ) { for (key, name_resolution) in resolver.as_ref().resolutions(self).iter() { - let name_resolution = name_resolution.borrow(); + let name_resolution = name_resolution.borrow(resolver.as_ref()); if let Some(decl) = name_resolution.best_decl() { f(resolver, key.ident, name_resolution.orig_ident_span, key.ns, decl); } @@ -806,7 +807,7 @@ impl<'ra> Module<'ra> { mut f: impl FnMut(&mut R, IdentKey, Span, Namespace, Decl<'ra>), ) { for (key, name_resolution) in resolver.as_mut().resolutions(self).iter() { - let name_resolution = name_resolution.borrow(); + let name_resolution = name_resolution.borrow(resolver.as_mut()); if let Some(decl) = name_resolution.best_decl() { f(resolver, key.ident, name_resolution.orig_ident_span, key.ns, decl); } @@ -1252,10 +1253,11 @@ impl<'ra> DeclData<'ra> { /// the declaration may not be as "determined" as we think. /// FIXME: relationship between this function and similar `NameResolution::determined_decl` /// is unclear. - fn determined(&self) -> bool { + fn determined<'tcx>(&self, r: &Resolver<'ra, 'tcx>) -> bool { match &self.kind { DeclKind::Import { source_decl, import, .. } if import.is_glob() => { - !import.parent_scope.module.has_unexpanded_invocations() && source_decl.determined() + !import.parent_scope.module.has_unexpanded_invocations(r) + && source_decl.determined(r) } _ => true, } @@ -1336,7 +1338,7 @@ pub struct Resolver<'ra, 'tcx> { graph_root: LocalModule<'ra>, /// Assert that we are in speculative resolution mode (unsafe field). - assert_speculative: bool, + speculative_flag: SpeculativeFlag, prelude: Option> = None, extern_prelude: FxIndexMap>, @@ -1810,7 +1812,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // The outermost module has def ID 0; this is not reflected in the // AST. graph_root, - assert_speculative: false, // Only set/cleared in Resolver::resolve_imports for now + // Only set/cleared in Resolver::resolve_imports for now + speculative_flag: SpeculativeFlag::default(), extern_prelude, empty_module, @@ -2011,7 +2014,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { /// Returns a conditionally mutable resolver that can be mutated. /// Will panic if the `assert_speculative` field is true. fn cm_mut(&mut self) -> CmResolver<'_, 'ra, 'tcx> { - assert!(!self.assert_speculative, "can't mutably borrow speculative resolver"); + assert!( + !self.speculative_flag.is_speculative(), + "can't mutably borrow speculative resolver" + ); CmResolver::Mut(self) } @@ -2127,7 +2133,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { found_traits: &mut Vec>, ) { module.ensure_traits(self); - let traits = module.traits.borrow(); + let traits = module.traits.borrow(self); for &(trait_name, trait_binding, trait_module, lint_ambiguous) in traits.as_ref().unwrap().iter() { @@ -2178,7 +2184,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { fn resolutions(&self, module: Module<'ra>) -> CmRef<'ra, ResolutionTable<'ra>> { match &module.0.0.lazy_resolutions { - Resolutions::Local(local_res) => CmRef::Tracked(local_res.borrow()), + Resolutions::Local(local_res) => local_res.borrow(self), Resolutions::Extern(extern_res) => { // It is fine to return a `CmRef::Untracked`, we never give out a `&mut` // to an external table. @@ -2206,8 +2212,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { &self, module: Module<'ra>, key: BindingKey, - ) -> Option>> { - self.resolutions(module).get(&key).map(|resolution| resolution.0.borrow()) + ) -> Option>> { + self.resolutions(module).get(&key).map(|resolution| resolution.0.borrow(self)) } #[track_caller] @@ -2917,7 +2923,7 @@ mod ref_mut { } pub(crate) fn set<'ra, 'tcx>(&self, val: T, r: &Resolver<'ra, 'tcx>) { - if r.assert_speculative { + if r.speculative_flag.is_speculative() { panic!("not allowed to mutate a `CmCell` during speculative resolution") } self.0.set(val); @@ -2946,6 +2952,27 @@ mod ref_mut { } } + pub(crate) mod speculative { + #[derive(Debug, Clone, Copy, Default)] + pub(crate) struct SpeculativeFlag(bool); + + impl SpeculativeFlag { + /// # SAFETY + /// + /// All borrows created by `CmRefCell::borrow` must be dropped before changing + /// the speculative flag: + /// - `tracked` borrows before setting it to `true`. + /// - `untracked` borrows before setting it to `false`. + pub(crate) unsafe fn set(&mut self, value: bool) { + self.0 = value; + } + + pub(crate) fn is_speculative(&self) -> bool { + self.0 + } + } + } + /// A wrapper around a [`RefCell`] that only allows writes (mutable borrows) based on a condition in the resolver. #[derive(Default)] pub(crate) struct CmRefCell(RefCell); @@ -2965,21 +2992,42 @@ mod ref_mut { &self, r: &Resolver<'ra, 'tcx>, ) -> Result, BorrowMutError> { - if r.assert_speculative { + if r.speculative_flag.is_speculative() { panic!("not allowed to mutably borrow a `CmRefCell` during speculative resolution"); } self.0.try_borrow_mut() } #[track_caller] - pub(crate) fn borrow(&self) -> Ref<'_, T> { - self.0.borrow() + pub(crate) fn borrow<'ra, 'tcx>(&self, r: &Resolver<'ra, 'tcx>) -> CmRef<'_, T> { + if r.speculative_flag.is_speculative() { + // `try_borrow_unguarded` is unsafe because it returns a `&T` instead + // of `Ref<'_, T>`. It does provides an extra check to make sure no live + // `RefMut`s are still alive, but the other way can not be checked, so: + // + // SAFETY: This is only safe because we know that every `Untracked` borrow + // is only created during the import resolutions phase: + // + // ```rust + // // tracked borrows + // unsafe { resolver.speculative_flag.set_true() }; + // import_resolution(); // untracked borrows + // unsafe { resolver.speculative_flag.set_true() }; + // // tracked borrows + // ``` + // + // `speculative::Flag` requires all of the borrows that happened during a + // particular phase are dropped before being set to true/false. + CmRef::Untracked(unsafe { self.0.try_borrow_unguarded().unwrap() }) + } else { + CmRef::Tracked(self.0.borrow()) + } } } impl CmRefCell { pub(crate) fn take<'ra, 'tcx>(&self, r: &Resolver<'ra, 'tcx>) -> T { - if r.assert_speculative { + if r.speculative_flag.is_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 index 1e9d60ca21551..6921d0ed595fe 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -562,7 +562,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { star_span: Span, ) -> Result)>, Indeterminate> { let target_trait = self.expect_module(trait_def_id); - if target_trait.has_unexpanded_invocations() { + if target_trait.has_unexpanded_invocations(self) { return Err(Indeterminate); } // FIXME: Instead of waiting try generating all trait methods, and pruning