Make untracked.cstore lockable so that resolution can still write to it when using TyCtxt

Author: oli-obk

compiler/rustc_interface/src/passes.rs

@@ -597,7 +597,7 @@ fn output_filenames(tcx: TyCtxt<'_>, (): ()) -> Arc<OutputFilenames> {
         }
     }
 
-    write_out_deps(sess, tcx.cstore_untracked(), &outputs, &output_paths);
+    write_out_deps(sess, &*tcx.cstore_untracked(), &outputs, &output_paths);
 
     let only_dep_info = sess.opts.output_types.contains_key(&OutputType::DepInfo)
         && sess.opts.output_types.len() == 1;

compiler/rustc_interface/src/queries.rs

@@ -215,6 +215,9 @@ impl<'tcx> Queries<'tcx> {
                 ast_lowering: untracked_resolver_for_lowering,
             } = resolver_outputs;
 
+            // Make sure we don't mutate the cstore from here on.
+            std::mem::forget(untracked.cstore.read());
+
             let gcx = passes::create_global_ctxt(
                 self.compiler,
                 lint_store,

compiler/rustc_metadata/src/creader.rs

@@ -8,7 +8,7 @@ use rustc_ast::expand::allocator::AllocatorKind;
 use rustc_ast::{self as ast, *};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::svh::Svh;
-use rustc_data_structures::sync::ReadGuard;
+use rustc_data_structures::sync::{MappedReadGuard, ReadGuard};
 use rustc_expand::base::SyntaxExtension;
 use rustc_hir::def_id::{CrateNum, LocalDefId, StableCrateId, LOCAL_CRATE};
 use rustc_hir::definitions::Definitions;
@@ -127,11 +127,10 @@ impl<'a> std::fmt::Debug for CrateDump<'a> {
 }
 
 impl CStore {
-    pub fn from_tcx(tcx: TyCtxt<'_>) -> &CStore {
-        tcx.cstore_untracked()
-            .as_any()
-            .downcast_ref::<CStore>()
-            .expect("`tcx.cstore` is not a `CStore`")
+    pub fn from_tcx(tcx: TyCtxt<'_>) -> MappedReadGuard<'_, CStore> {
+        MappedReadGuard::map(tcx.cstore_untracked(), |c| {
+            c.as_any().downcast_ref::<CStore>().expect("`tcx.cstore` is not a `CStore`")
+        })
     }
 
     fn alloc_new_crate_num(&mut self) -> CrateNum {

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -130,7 +130,13 @@ macro_rules! provide_one {
                 $tcx.ensure().crate_hash($def_id.krate);
             }
 
-            let $cdata = CStore::from_tcx($tcx).get_crate_data($def_id.krate);
+            let cdata = rustc_data_structures::sync::MappedReadGuard::map(CStore::from_tcx($tcx), |c| {
+                c.get_crate_data($def_id.krate).cdata
+            });
+            let $cdata = crate::creader::CrateMetadataRef {
+                cdata: &cdata,
+                cstore: &CStore::from_tcx($tcx),
+            };
 
             $compute
         }

compiler/rustc_middle/src/ty/context.rs

@@ -36,7 +36,7 @@ use rustc_data_structures::profiling::SelfProfilerRef;
 use rustc_data_structures::sharded::{IntoPointer, ShardedHashMap};
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::steal::Steal;
-use rustc_data_structures::sync::{self, Lock, Lrc, ReadGuard, WorkerLocal};
+use rustc_data_structures::sync::{self, Lock, Lrc, MappedReadGuard, ReadGuard, WorkerLocal};
 use rustc_errors::{
     DecorateLint, DiagnosticBuilder, DiagnosticMessage, ErrorGuaranteed, MultiSpan,
 };
@@ -836,7 +836,7 @@ impl<'tcx> TyCtxt<'tcx> {
         if let Some(id) = id.as_local() {
             self.definitions_untracked().def_key(id)
         } else {
-            self.untracked.cstore.def_key(id)
+            self.cstore_untracked().def_key(id)
         }
     }
 
@@ -850,7 +850,7 @@ impl<'tcx> TyCtxt<'tcx> {
         if let Some(id) = id.as_local() {
             self.definitions_untracked().def_path(id)
         } else {
-            self.untracked.cstore.def_path(id)
+            self.cstore_untracked().def_path(id)
         }
     }
 
@@ -860,7 +860,7 @@ impl<'tcx> TyCtxt<'tcx> {
         if let Some(def_id) = def_id.as_local() {
             self.definitions_untracked().def_path_hash(def_id)
         } else {
-            self.untracked.cstore.def_path_hash(def_id)
+            self.cstore_untracked().def_path_hash(def_id)
         }
     }
 
@@ -869,7 +869,7 @@ impl<'tcx> TyCtxt<'tcx> {
         if crate_num == LOCAL_CRATE {
             self.sess.local_stable_crate_id()
         } else {
-            self.untracked.cstore.stable_crate_id(crate_num)
+            self.cstore_untracked().stable_crate_id(crate_num)
         }
     }
 
@@ -880,7 +880,7 @@ impl<'tcx> TyCtxt<'tcx> {
         if stable_crate_id == self.sess.local_stable_crate_id() {
             LOCAL_CRATE
         } else {
-            self.untracked.cstore.stable_crate_id_to_crate_num(stable_crate_id)
+            self.cstore_untracked().stable_crate_id_to_crate_num(stable_crate_id)
         }
     }
 
@@ -899,7 +899,7 @@ impl<'tcx> TyCtxt<'tcx> {
         } else {
             // If this is a DefPathHash from an upstream crate, let the CrateStore map
             // it to a DefId.
-            let cstore = &*self.untracked.cstore;
+            let cstore = &*self.cstore_untracked();
             let cnum = cstore.stable_crate_id_to_crate_num(stable_crate_id);
             cstore.def_path_hash_to_def_id(cnum, hash)
         }
@@ -913,7 +913,7 @@ impl<'tcx> TyCtxt<'tcx> {
         let (crate_name, stable_crate_id) = if def_id.is_local() {
             (self.crate_name(LOCAL_CRATE), self.sess.local_stable_crate_id())
         } else {
-            let cstore = &*self.untracked.cstore;
+            let cstore = &*self.cstore_untracked();
             (cstore.crate_name(def_id.krate), cstore.stable_crate_id(def_id.krate))
         };
 
@@ -1011,8 +1011,8 @@ impl<'tcx> TyCtxt<'tcx> {
 
     /// Note that this is *untracked* and should only be used within the query
     /// system if the result is otherwise tracked through queries
-    pub fn cstore_untracked(self) -> &'tcx CrateStoreDyn {
-        &*self.untracked.cstore
+    pub fn cstore_untracked(self) -> MappedReadGuard<'tcx, CrateStoreDyn> {
+        ReadGuard::map(self.untracked.cstore.read(), |c| &**c)
     }
 
     /// Note that this is *untracked* and should only be used within the query

compiler/rustc_query_system/src/ich/hcx.rs

@@ -90,7 +90,7 @@ impl<'a> StableHashingContext<'a> {
         if let Some(def_id) = def_id.as_local() {
             self.local_def_path_hash(def_id)
         } else {
-            self.untracked.cstore.def_path_hash(def_id)
+            self.untracked.cstore.read().def_path_hash(def_id)
         }
     }
 

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -130,11 +130,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                         def_key.disambiguated_data.data.get_opt_name().expect("module without name")
                     };
 
+                    let expn_id = self.cstore().module_expansion_untracked(def_id, &self.tcx.sess);
+                    let span = self.cstore().get_span_untracked(def_id, &self.tcx.sess);
                     Some(self.new_module(
                         parent,
                         ModuleKind::Def(def_kind, def_id, name),
-                        self.cstore().module_expansion_untracked(def_id, &self.tcx.sess),
-                        self.cstore().get_span_untracked(def_id, &self.tcx.sess),
+                        expn_id,
+                        span,
                         // FIXME: Account for `#[no_implicit_prelude]` attributes.
                         parent.map_or(false, |module| module.no_implicit_prelude),
                     ))
@@ -179,7 +181,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             return macro_data.clone();
         }
 
-        let (ext, macro_rules) = match self.cstore().load_macro_untracked(def_id, &self.tcx.sess) {
+        let load_macro_untracked = self.cstore().load_macro_untracked(def_id, &self.tcx.sess);
+        let (ext, macro_rules) = match load_macro_untracked {
             LoadedMacro::MacroDef(item, edition) => (
                 Lrc::new(self.compile_macro(&item, edition).0),
                 matches!(item.kind, ItemKind::MacroDef(def) if def.macro_rules),
@@ -204,9 +207,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     }
 
     pub(crate) fn build_reduced_graph_external(&mut self, module: Module<'a>) {
-        for child in
-            Vec::from_iter(self.cstore().module_children_untracked(module.def_id(), self.tcx.sess))
-        {
+        let children =
+            Vec::from_iter(self.cstore().module_children_untracked(module.def_id(), self.tcx.sess));
+        for child in children {
             let parent_scope = ParentScope::module(module, self);
             BuildReducedGraphVisitor { r: self, parent_scope }
                 .build_reduced_graph_for_external_crate_res(child);
@@ -1000,23 +1003,26 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
             | Res::Err => bug!("unexpected resolution: {:?}", res),
         }
         // Record some extra data for better diagnostics.
-        let cstore = self.r.cstore();
         match res {
             Res::Def(DefKind::Struct, def_id) => {
+                let cstore = self.r.cstore();
                 if let Some((ctor_kind, ctor_def_id)) = cstore.ctor_untracked(def_id) {
                     let ctor_res = Res::Def(DefKind::Ctor(CtorOf::Struct, ctor_kind), ctor_def_id);
                     let ctor_vis = cstore.visibility_untracked(ctor_def_id);
                     let field_visibilities =
                         cstore.struct_field_visibilities_untracked(def_id).collect();
+                    drop(cstore);
                     self.r
                         .struct_constructors
                         .insert(def_id, (ctor_res, ctor_vis, field_visibilities));
+                } else {
+                    drop(cstore);
                 }
                 self.insert_field_names_extern(def_id)
             }
             Res::Def(DefKind::Union, def_id) => self.insert_field_names_extern(def_id),
             Res::Def(DefKind::AssocFn, def_id) => {
-                if cstore.fn_has_self_parameter_untracked(def_id, self.r.tcx.sess) {
+                if self.r.cstore().fn_has_self_parameter_untracked(def_id, self.r.tcx.sess) {
                     self.r.has_self.insert(def_id);
                 }
             }

compiler/rustc_resolve/src/lib.rs

@@ -27,7 +27,7 @@ use rustc_ast::{self as ast, NodeId, CRATE_NODE_ID};
 use rustc_ast::{AngleBracketedArg, Crate, Expr, ExprKind, GenericArg, GenericArgs, LitKind, Path};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_data_structures::intern::Interned;
-use rustc_data_structures::sync::{Lrc, RwLock};
+use rustc_data_structures::sync::{Lrc, MappedReadGuard, ReadGuard, RwLock};
 use rustc_errors::{Applicability, DiagnosticBuilder, ErrorGuaranteed};
 use rustc_expand::base::{DeriveResolutions, SyntaxExtension, SyntaxExtensionKind};
 use rustc_hir::def::Namespace::{self, *};
@@ -1132,7 +1132,7 @@ impl DefIdTree for ResolverTree<'_> {
         let ResolverTree(Untracked { definitions, cstore, .. }) = self;
         match id.as_local() {
             Some(id) => definitions.read().def_key(id).parent,
-            None => cstore.as_any().downcast_ref::<CStore>().unwrap().def_key(id).parent,
+            None => cstore.read().as_any().downcast_ref::<CStore>().unwrap().def_key(id).parent,
         }
         .map(|index| DefId { index, ..id })
     }
@@ -1328,7 +1328,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             local_crate_name: crate_name,
             used_extern_options: Default::default(),
             untracked: Untracked {
-                cstore: Box::new(CStore::new(session)),
+                cstore: RwLock::new(Box::new(CStore::new(session))),
                 source_span,
                 definitions: RwLock::new(definitions),
             },
@@ -1487,14 +1487,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             &self.tcx.sess,
             &*self.metadata_loader,
             self.local_crate_name,
-            &mut *self.untracked.cstore.untracked_as_any().downcast_mut().unwrap(),
+            &mut *self.untracked.cstore.write().untracked_as_any().downcast_mut().unwrap(),
             self.untracked.definitions.read(),
             &mut self.used_extern_options,
         ))
     }
 
-    fn cstore(&self) -> &CStore {
-        self.untracked.cstore.as_any().downcast_ref().unwrap()
+    fn cstore(&self) -> MappedReadGuard<'_, CStore> {
+        ReadGuard::map(self.untracked.cstore.read(), |r| r.as_any().downcast_ref().unwrap())
     }
 
     fn dummy_ext(&self, macro_kind: MacroKind) -> Lrc<SyntaxExtension> {

compiler/rustc_session/src/cstore.rs

@@ -205,7 +205,7 @@ pub trait MetadataLoader {
     fn get_dylib_metadata(&self, target: &Target, filename: &Path) -> Result<MetadataRef, String>;
 }
 
-pub type MetadataLoaderDyn = dyn MetadataLoader + Sync;
+pub type MetadataLoaderDyn = dyn MetadataLoader + Send + Sync;
 
 /// A store of Rust crates, through which their metadata can be accessed.
 ///
@@ -250,11 +250,11 @@ pub trait CrateStore: std::fmt::Debug {
     fn import_source_files(&self, sess: &Session, cnum: CrateNum);
 }
 
-pub type CrateStoreDyn = dyn CrateStore + sync::Sync;
+pub type CrateStoreDyn = dyn CrateStore + sync::Sync + sync::Send;
 
 #[derive(Debug)]
 pub struct Untracked {
-    pub cstore: Box<CrateStoreDyn>,
+    pub cstore: RwLock<Box<CrateStoreDyn>>,
     /// Reference span for definitions.
     pub source_span: IndexVec<LocalDefId, Span>,
     pub definitions: RwLock<Definitions>,