From 10be74569ce3f3140e2d035af920ee0e78682407 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 26 Mar 2024 16:42:02 +0000 Subject: [PATCH 1/5] Create new `CrateNum` only after knowing it's going to succeed --- compiler/rustc_metadata/src/creader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 784fd4b3a3b72..1dbfba7e504fa 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -167,7 +167,6 @@ impl CStore { fn intern_stable_crate_id(&mut self, root: &CrateRoot) -> Result { assert_eq!(self.metas.len(), self.stable_crate_ids.len()); - let num = CrateNum::new(self.stable_crate_ids.len()); if let Some(&existing) = self.stable_crate_ids.get(&root.stable_crate_id()) { // Check for (potential) conflicts with the local crate if existing == LOCAL_CRATE { @@ -181,6 +180,7 @@ impl CStore { } } else { self.metas.push(None); + let num = CrateNum::new(self.stable_crate_ids.len()); self.stable_crate_ids.insert(root.stable_crate_id(), num); Ok(num) } From fbc9b94064c611f341d613ce14d321b267b3c298 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 26 Mar 2024 16:52:06 +0000 Subject: [PATCH 2/5] Move `stable_crate_ids` from `CrateStore` to `Untracked` This way it's like `Definitions`, which creates `DefId`s by interning `DefPathData`s, but for interning stable crate hashes --- compiler/rustc_interface/src/queries.rs | 14 ++++++--- compiler/rustc_metadata/src/creader.rs | 31 +++++++++---------- .../src/rmeta/decoder/cstore_impl.rs | 15 +++++---- compiler/rustc_middle/src/ty/context.rs | 7 ++++- compiler/rustc_session/src/cstore.rs | 7 +++-- 5 files changed, 42 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index ee677a092e2f1..375a390948b34 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -8,7 +8,7 @@ use rustc_codegen_ssa::CodegenResults; use rustc_data_structures::steal::Steal; use rustc_data_structures::svh::Svh; use rustc_data_structures::sync::{AppendOnlyIndexVec, FreezeLock, OnceLock, WorkerLocal}; -use rustc_hir::def_id::{StableCrateId, LOCAL_CRATE}; +use rustc_hir::def_id::{StableCrateId, StableCrateIdMap, LOCAL_CRATE}; use rustc_hir::definitions::Definitions; use rustc_incremental::setup_dep_graph; use rustc_metadata::creader::CStore; @@ -140,11 +140,17 @@ impl<'tcx> Queries<'tcx> { let cstore = FreezeLock::new(Box::new(CStore::new( self.compiler.codegen_backend.metadata_loader(), - stable_crate_id, )) as _); let definitions = FreezeLock::new(Definitions::new(stable_crate_id)); - let untracked = - Untracked { cstore, source_span: AppendOnlyIndexVec::new(), definitions }; + + let mut stable_crate_ids = StableCrateIdMap::default(); + stable_crate_ids.insert(stable_crate_id, LOCAL_CRATE); + let untracked = Untracked { + cstore, + source_span: AppendOnlyIndexVec::new(), + definitions, + stable_crate_ids: FreezeLock::new(stable_crate_ids), + }; let qcx = passes::create_global_ctxt( self.compiler, diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 1dbfba7e504fa..5083b75174cf6 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -13,7 +13,7 @@ use rustc_data_structures::sync::{self, FreezeReadGuard, FreezeWriteGuard}; use rustc_errors::DiagCtxt; use rustc_expand::base::SyntaxExtension; use rustc_fs_util::try_canonicalize; -use rustc_hir::def_id::{CrateNum, LocalDefId, StableCrateId, StableCrateIdMap, LOCAL_CRATE}; +use rustc_hir::def_id::{CrateNum, LocalDefId, StableCrateId, LOCAL_CRATE}; use rustc_hir::definitions::Definitions; use rustc_index::IndexVec; use rustc_middle::ty::TyCtxt; @@ -62,9 +62,6 @@ pub struct CStore { /// This crate has a `#[alloc_error_handler]` item. has_alloc_error_handler: bool, - /// The interned [StableCrateId]s. - pub(crate) stable_crate_ids: StableCrateIdMap, - /// Unused externs of the crate unused_externs: Vec, } @@ -165,9 +162,15 @@ impl CStore { }) } - fn intern_stable_crate_id(&mut self, root: &CrateRoot) -> Result { - assert_eq!(self.metas.len(), self.stable_crate_ids.len()); - if let Some(&existing) = self.stable_crate_ids.get(&root.stable_crate_id()) { + fn intern_stable_crate_id<'tcx>( + &mut self, + root: &CrateRoot, + tcx: TyCtxt<'tcx>, + ) -> Result { + assert_eq!(self.metas.len(), tcx.untracked().stable_crate_ids.read().len()); + if let Some(&existing) = + tcx.untracked().stable_crate_ids.read().get(&root.stable_crate_id()) + { // Check for (potential) conflicts with the local crate if existing == LOCAL_CRATE { Err(CrateError::SymbolConflictsCurrent(root.name())) @@ -180,8 +183,8 @@ impl CStore { } } else { self.metas.push(None); - let num = CrateNum::new(self.stable_crate_ids.len()); - self.stable_crate_ids.insert(root.stable_crate_id(), num); + let num = CrateNum::new(tcx.untracked().stable_crate_ids.read().len()); + tcx.untracked().stable_crate_ids.write().insert(root.stable_crate_id(), num); Ok(num) } } @@ -289,12 +292,7 @@ impl CStore { } } - pub fn new( - metadata_loader: Box, - local_stable_crate_id: StableCrateId, - ) -> CStore { - let mut stable_crate_ids = StableCrateIdMap::default(); - stable_crate_ids.insert(local_stable_crate_id, LOCAL_CRATE); + pub fn new(metadata_loader: Box) -> CStore { CStore { metadata_loader, // We add an empty entry for LOCAL_CRATE (which maps to zero) in @@ -307,7 +305,6 @@ impl CStore { alloc_error_handler_kind: None, has_global_allocator: false, has_alloc_error_handler: false, - stable_crate_ids, unused_externs: Vec::new(), } } @@ -416,7 +413,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { let private_dep = self.is_private_dep(name.as_str(), private_dep); // Claim this crate number and cache it - let cnum = self.cstore.intern_stable_crate_id(&crate_root)?; + let cnum = self.cstore.intern_stable_crate_id(&crate_root, self.tcx)?; info!( "register crate `{}` (cnum = {}. private_dep = {})", diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 2935d5b8f6328..531b2e05411a0 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -629,13 +629,6 @@ impl CrateStore for CStore { self.get_crate_data(cnum).root.stable_crate_id } - fn stable_crate_id_to_crate_num(&self, stable_crate_id: StableCrateId) -> CrateNum { - *self - .stable_crate_ids - .get(&stable_crate_id) - .unwrap_or_else(|| bug!("uninterned StableCrateId: {stable_crate_id:?}")) - } - /// Returns the `DefKey` for a given `DefId`. This indicates the /// parent `DefId` as well as some idea of what kind of data the /// `DefId` refers to. @@ -657,7 +650,13 @@ fn provide_cstore_hooks(providers: &mut Providers) { // If this is a DefPathHash from an upstream crate, let the CrateStore map // it to a DefId. let cstore = CStore::from_tcx(tcx.tcx); - let cnum = cstore.stable_crate_id_to_crate_num(stable_crate_id); + let cnum = *tcx + .untracked() + .stable_crate_ids + .read() + .get(&stable_crate_id) + .unwrap_or_else(|| bug!("uninterned StableCrateId: {stable_crate_id:?}")); + assert_ne!(cnum, LOCAL_CRATE); let def_index = cstore.get_crate_data(cnum).def_path_hash_to_def_index(hash); DefId { krate: cnum, index: def_index } }; diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 6275c5d2a1194..d30a4ac6a9cbd 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -1140,7 +1140,12 @@ impl<'tcx> TyCtxt<'tcx> { if stable_crate_id == self.stable_crate_id(LOCAL_CRATE) { LOCAL_CRATE } else { - self.cstore_untracked().stable_crate_id_to_crate_num(stable_crate_id) + *self + .untracked() + .stable_crate_ids + .read() + .get(&stable_crate_id) + .unwrap_or_else(|| bug!("uninterned StableCrateId: {stable_crate_id:?}")) } } diff --git a/compiler/rustc_session/src/cstore.rs b/compiler/rustc_session/src/cstore.rs index cb6656bae062c..83377b66095d6 100644 --- a/compiler/rustc_session/src/cstore.rs +++ b/compiler/rustc_session/src/cstore.rs @@ -6,7 +6,9 @@ use crate::search_paths::PathKind; use crate::utils::NativeLibKind; use rustc_ast as ast; use rustc_data_structures::sync::{self, AppendOnlyIndexVec, FreezeLock}; -use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, StableCrateId, LOCAL_CRATE}; +use rustc_hir::def_id::{ + CrateNum, DefId, LocalDefId, StableCrateId, StableCrateIdMap, LOCAL_CRATE, +}; use rustc_hir::definitions::{DefKey, DefPath, DefPathHash, Definitions}; use rustc_span::symbol::Symbol; use rustc_span::Span; @@ -217,7 +219,6 @@ pub trait CrateStore: std::fmt::Debug { // incr. comp. uses to identify a CrateNum. fn crate_name(&self, cnum: CrateNum) -> Symbol; fn stable_crate_id(&self, cnum: CrateNum) -> StableCrateId; - fn stable_crate_id_to_crate_num(&self, stable_crate_id: StableCrateId) -> CrateNum; } pub type CrateStoreDyn = dyn CrateStore + sync::DynSync + sync::DynSend; @@ -227,4 +228,6 @@ pub struct Untracked { /// Reference span for definitions. pub source_span: AppendOnlyIndexVec, pub definitions: FreezeLock, + /// The interned [StableCrateId]s. + pub stable_crate_ids: FreezeLock, } From 0025c9cc5001c194ac1f8ce4824d93a0efbe5c18 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 26 Mar 2024 16:59:14 +0000 Subject: [PATCH 3/5] Isolate `CrateNum` creation to `TyCtxt` methods --- compiler/rustc_metadata/src/creader.rs | 20 ++++++++------------ compiler/rustc_middle/src/ty/context.rs | 10 ++++++++++ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 5083b75174cf6..83fe39baf1fab 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -168,25 +168,21 @@ impl CStore { tcx: TyCtxt<'tcx>, ) -> Result { assert_eq!(self.metas.len(), tcx.untracked().stable_crate_ids.read().len()); - if let Some(&existing) = - tcx.untracked().stable_crate_ids.read().get(&root.stable_crate_id()) - { + let num = tcx.create_crate_num(root.stable_crate_id()).map_err(|existing| { // Check for (potential) conflicts with the local crate if existing == LOCAL_CRATE { - Err(CrateError::SymbolConflictsCurrent(root.name())) + CrateError::SymbolConflictsCurrent(root.name()) } else if let Some(crate_name1) = self.metas[existing].as_ref().map(|data| data.name()) { let crate_name0 = root.name(); - Err(CrateError::StableCrateIdCollision(crate_name0, crate_name1)) + CrateError::StableCrateIdCollision(crate_name0, crate_name1) } else { - Err(CrateError::NotFound(root.name())) + CrateError::NotFound(root.name()) } - } else { - self.metas.push(None); - let num = CrateNum::new(tcx.untracked().stable_crate_ids.read().len()); - tcx.untracked().stable_crate_ids.write().insert(root.stable_crate_id(), num); - Ok(num) - } + })?; + + self.metas.push(None); + Ok(num) } pub fn has_crate_data(&self, cnum: CrateNum) -> bool { diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index d30a4ac6a9cbd..3832fecfee4e9 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -1261,6 +1261,16 @@ impl<'tcx> TyCtxt<'tcx> { feed } + pub fn create_crate_num(self, stable_crate_id: StableCrateId) -> Result { + if let Some(&existing) = self.untracked().stable_crate_ids.read().get(&stable_crate_id) { + return Err(existing); + } + + let num = CrateNum::new(self.untracked().stable_crate_ids.read().len()); + self.untracked().stable_crate_ids.write().insert(stable_crate_id, num); + Ok(num) + } + pub fn iter_local_def_id(self) -> impl Iterator + 'tcx { // Create a dependency to the red node to be sure we re-execute this when the amount of // definitions change. From 9e63e991e9690122df470f1fd6931a99bcc5e324 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 26 Mar 2024 17:01:47 +0000 Subject: [PATCH 4/5] Prepare for `CrateNum` query feeding on creation --- compiler/rustc_metadata/src/creader.rs | 7 ++++--- compiler/rustc_middle/src/ty/context.rs | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 83fe39baf1fab..888c2427d8f46 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -16,7 +16,7 @@ use rustc_fs_util::try_canonicalize; use rustc_hir::def_id::{CrateNum, LocalDefId, StableCrateId, LOCAL_CRATE}; use rustc_hir::definitions::Definitions; use rustc_index::IndexVec; -use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::{TyCtxt, TyCtxtFeed}; use rustc_session::config::{self, CrateType, ExternLocation}; use rustc_session::cstore::{CrateDepKind, CrateSource, ExternCrate, ExternCrateSource}; use rustc_session::lint; @@ -166,7 +166,7 @@ impl CStore { &mut self, root: &CrateRoot, tcx: TyCtxt<'tcx>, - ) -> Result { + ) -> Result, CrateError> { assert_eq!(self.metas.len(), tcx.untracked().stable_crate_ids.read().len()); let num = tcx.create_crate_num(root.stable_crate_id()).map_err(|existing| { // Check for (potential) conflicts with the local crate @@ -409,7 +409,8 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { let private_dep = self.is_private_dep(name.as_str(), private_dep); // Claim this crate number and cache it - let cnum = self.cstore.intern_stable_crate_id(&crate_root, self.tcx)?; + let feed = self.cstore.intern_stable_crate_id(&crate_root, self.tcx)?; + let cnum = feed.key(); info!( "register crate `{}` (cnum = {}. private_dep = {})", diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 3832fecfee4e9..b5769e6fee25c 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -1261,14 +1261,17 @@ impl<'tcx> TyCtxt<'tcx> { feed } - pub fn create_crate_num(self, stable_crate_id: StableCrateId) -> Result { + pub fn create_crate_num( + self, + stable_crate_id: StableCrateId, + ) -> Result, CrateNum> { if let Some(&existing) = self.untracked().stable_crate_ids.read().get(&stable_crate_id) { return Err(existing); } let num = CrateNum::new(self.untracked().stable_crate_ids.read().len()); self.untracked().stable_crate_ids.write().insert(stable_crate_id, num); - Ok(num) + Ok(TyCtxtFeed { key: num, tcx: self }) } pub fn iter_local_def_id(self) -> impl Iterator + 'tcx { From e9a2f8fef181a8aa0145a2665804916effb6365e Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 27 Mar 2024 08:07:23 +0000 Subject: [PATCH 5/5] Remove `feed_local_crate` in favor of creating the `CrateNum` via `TyCtxt` --- compiler/rustc_interface/src/queries.rs | 8 ++++---- compiler/rustc_middle/src/ty/context.rs | 7 ------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index 375a390948b34..1b9165342d45d 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -143,13 +143,12 @@ impl<'tcx> Queries<'tcx> { )) as _); let definitions = FreezeLock::new(Definitions::new(stable_crate_id)); - let mut stable_crate_ids = StableCrateIdMap::default(); - stable_crate_ids.insert(stable_crate_id, LOCAL_CRATE); + let stable_crate_ids = FreezeLock::new(StableCrateIdMap::default()); let untracked = Untracked { cstore, source_span: AppendOnlyIndexVec::new(), definitions, - stable_crate_ids: FreezeLock::new(stable_crate_ids), + stable_crate_ids, }; let qcx = passes::create_global_ctxt( @@ -164,7 +163,8 @@ impl<'tcx> Queries<'tcx> { ); qcx.enter(|tcx| { - let feed = tcx.feed_local_crate(); + let feed = tcx.create_crate_num(stable_crate_id).unwrap(); + assert_eq!(feed.key(), LOCAL_CRATE); feed.crate_name(crate_name); let feed = tcx.feed_unit_query(); diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index b5769e6fee25c..65ff643ed4b39 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -565,13 +565,6 @@ impl<'tcx> TyCtxt<'tcx> { TyCtxtFeed { tcx: self, key: () } } - /// Can only be fed before queries are run, and is thus exempt from any - /// incremental issues. Do not use except for the initial query feeding. - pub fn feed_local_crate(self) -> TyCtxtFeed<'tcx, CrateNum> { - self.dep_graph.assert_ignored(); - TyCtxtFeed { tcx: self, key: LOCAL_CRATE } - } - /// Only used in the resolver to register the `CRATE_DEF_ID` `DefId` and feed /// some queries for it. It will panic if used twice. pub fn create_local_crate_def_id(self, span: Span) -> TyCtxtFeed<'tcx, LocalDefId> {