From 32bd3c30d8d4d5909b9bafd4c7b554f46b58d9ab Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 26 Mar 2024 12:06:07 +0000 Subject: [PATCH 1/3] Start replacing `CStore` trait methods with hooks. This also avoids the cyclic definition issues with CrateStore being defined after TyCtxt, but needing to be used in TyCtxt. --- .../rustc_metadata/src/rmeta/decoder/cstore_impl.rs | 12 ++++++++---- compiler/rustc_middle/src/hooks/mod.rs | 11 ++++++++--- compiler/rustc_middle/src/query/on_disk_cache.rs | 4 +--- compiler/rustc_session/src/cstore.rs | 6 ------ 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index b69a295f010f3..187364d8b44cf 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -378,6 +378,7 @@ provide! { tcx, def_id, other, cdata, } pub(in crate::rmeta) fn provide(providers: &mut Providers) { + provide_cstore_hooks(providers); // FIXME(#44234) - almost all of these queries have no sub-queries and // therefore no actual inputs, they're just reading tables calculated in // resolve! Does this work? Unsure! That's what the issue is about @@ -664,11 +665,14 @@ impl CrateStore for CStore { ) -> ExpnId { self.get_crate_data(cnum).expn_hash_to_expn_id(sess, index_guess, hash) } +} - fn import_source_files(&self, sess: &Session, cnum: CrateNum) { - let cdata = self.get_crate_data(cnum); +fn provide_cstore_hooks(providers: &mut Providers) { + providers.hooks.import_source_files = |tcx, cnum| { + let cstore = CStore::from_tcx(tcx.tcx); + let cdata = cstore.get_crate_data(cnum); for file_index in 0..cdata.root.source_map.size() { - cdata.imported_source_file(file_index as u32, sess); + cdata.imported_source_file(file_index as u32, tcx.sess); } - } + }; } diff --git a/compiler/rustc_middle/src/hooks/mod.rs b/compiler/rustc_middle/src/hooks/mod.rs index aa2cddad0938c..b5267e42780c9 100644 --- a/compiler/rustc_middle/src/hooks/mod.rs +++ b/compiler/rustc_middle/src/hooks/mod.rs @@ -6,7 +6,7 @@ use crate::mir; use crate::query::TyCtxtAt; use crate::ty::{Ty, TyCtxt}; -use rustc_span::def_id::LocalDefId; +use rustc_span::def_id::{CrateNum, LocalDefId}; use rustc_span::DUMMY_SP; macro_rules! declare_hooks { @@ -16,7 +16,6 @@ macro_rules! declare_hooks { $( $(#[$attr])* #[inline(always)] - #[must_use] pub fn $name(self, $($arg: $K,)*) -> $V { self.at(DUMMY_SP).$name($($arg,)*) @@ -28,7 +27,6 @@ macro_rules! declare_hooks { $( $(#[$attr])* #[inline(always)] - #[must_use] #[instrument(level = "debug", skip(self), ret)] pub fn $name(self, $($arg: $K,)*) -> $V { @@ -83,4 +81,11 @@ declare_hooks! { /// You do not want to call this yourself, instead use the cached version /// via `mir_built` hook build_mir(key: LocalDefId) -> mir::Body<'tcx>; + + + /// Imports all `SourceFile`s from the given crate into the current session. + /// This normally happens automatically when we decode a `Span` from + /// that crate's metadata - however, the incr comp cache needs + /// to trigger this manually when decoding a foreign `Span` + hook import_source_files(key: CrateNum) -> (); } diff --git a/compiler/rustc_middle/src/query/on_disk_cache.rs b/compiler/rustc_middle/src/query/on_disk_cache.rs index 9c7c46f2ad24b..06c5440849ec1 100644 --- a/compiler/rustc_middle/src/query/on_disk_cache.rs +++ b/compiler/rustc_middle/src/query/on_disk_cache.rs @@ -492,9 +492,7 @@ impl<'a, 'tcx> CacheDecoder<'a, 'tcx> { // expansion, so we use `import_source_files` to ensure that the foreign // source files are actually imported before we call `source_file_by_stable_id`. if source_file_cnum != LOCAL_CRATE { - self.tcx - .cstore_untracked() - .import_source_files(self.tcx.sess, source_file_cnum); + self.tcx.import_source_files(source_file_cnum); } source_map diff --git a/compiler/rustc_session/src/cstore.rs b/compiler/rustc_session/src/cstore.rs index a0f5eb59b6a25..4e59cd09ec42e 100644 --- a/compiler/rustc_session/src/cstore.rs +++ b/compiler/rustc_session/src/cstore.rs @@ -230,12 +230,6 @@ pub trait CrateStore: std::fmt::Debug { index_guess: u32, hash: ExpnHash, ) -> ExpnId; - - /// Imports all `SourceFile`s from the given crate into the current session. - /// This normally happens automatically when we decode a `Span` from - /// that crate's metadata - however, the incr comp cache needs - /// to trigger this manually when decoding a foreign `Span` - fn import_source_files(&self, sess: &Session, cnum: CrateNum); } pub type CrateStoreDyn = dyn CrateStore + sync::DynSync + sync::DynSend; From 0f5911c62647601cbe6c81e935615a770e7fc3ff Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 26 Mar 2024 12:31:41 +0000 Subject: [PATCH 2/3] Move `CrateStore::expn_hash_to_expn_id` to a hook --- .../src/rmeta/decoder/cstore_impl.rs | 16 +++++----------- compiler/rustc_middle/src/hooks/mod.rs | 8 +++++++- compiler/rustc_middle/src/query/on_disk_cache.rs | 7 +------ compiler/rustc_session/src/cstore.rs | 9 --------- 4 files changed, 13 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 187364d8b44cf..1720a4642c599 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -21,7 +21,7 @@ use rustc_middle::ty::{self, TyCtxt}; use rustc_middle::util::Providers; use rustc_session::cstore::{CrateStore, ExternCrate}; use rustc_session::{Session, StableCrateId}; -use rustc_span::hygiene::{ExpnHash, ExpnId}; +use rustc_span::hygiene::ExpnId; use rustc_span::symbol::{kw, Symbol}; use rustc_span::Span; @@ -655,19 +655,13 @@ impl CrateStore for CStore { let def_index = self.get_crate_data(cnum).def_path_hash_to_def_index(hash); DefId { krate: cnum, index: def_index } } - - fn expn_hash_to_expn_id( - &self, - sess: &Session, - cnum: CrateNum, - index_guess: u32, - hash: ExpnHash, - ) -> ExpnId { - self.get_crate_data(cnum).expn_hash_to_expn_id(sess, index_guess, hash) - } } fn provide_cstore_hooks(providers: &mut Providers) { + providers.hooks.expn_hash_to_expn_id = |tcx, cnum, index_guess, hash| { + let cstore = CStore::from_tcx(tcx.tcx); + cstore.get_crate_data(cnum).expn_hash_to_expn_id(tcx.sess, index_guess, hash) + }; providers.hooks.import_source_files = |tcx, cnum| { let cstore = CStore::from_tcx(tcx.tcx); let cdata = cstore.get_crate_data(cnum); diff --git a/compiler/rustc_middle/src/hooks/mod.rs b/compiler/rustc_middle/src/hooks/mod.rs index b5267e42780c9..b0f08334c542f 100644 --- a/compiler/rustc_middle/src/hooks/mod.rs +++ b/compiler/rustc_middle/src/hooks/mod.rs @@ -7,7 +7,7 @@ use crate::mir; use crate::query::TyCtxtAt; use crate::ty::{Ty, TyCtxt}; use rustc_span::def_id::{CrateNum, LocalDefId}; -use rustc_span::DUMMY_SP; +use rustc_span::{ExpnHash, ExpnId, DUMMY_SP}; macro_rules! declare_hooks { ($($(#[$attr:meta])*hook $name:ident($($arg:ident: $K:ty),*) -> $V:ty;)*) => { @@ -88,4 +88,10 @@ declare_hooks! { /// that crate's metadata - however, the incr comp cache needs /// to trigger this manually when decoding a foreign `Span` hook import_source_files(key: CrateNum) -> (); + + hook expn_hash_to_expn_id( + cnum: CrateNum, + index_guess: u32, + hash: ExpnHash + ) -> ExpnId; } diff --git a/compiler/rustc_middle/src/query/on_disk_cache.rs b/compiler/rustc_middle/src/query/on_disk_cache.rs index 06c5440849ec1..58df950ff16ad 100644 --- a/compiler/rustc_middle/src/query/on_disk_cache.rs +++ b/compiler/rustc_middle/src/query/on_disk_cache.rs @@ -632,12 +632,7 @@ impl<'a, 'tcx> SpanDecoder for CacheDecoder<'a, 'tcx> { expn_id } else { let index_guess = self.foreign_expn_data[&hash]; - self.tcx.cstore_untracked().expn_hash_to_expn_id( - self.tcx.sess, - krate, - index_guess, - hash, - ) + self.tcx.expn_hash_to_expn_id(krate, index_guess, hash) }; debug_assert_eq!(expn_id.krate, krate); diff --git a/compiler/rustc_session/src/cstore.rs b/compiler/rustc_session/src/cstore.rs index 4e59cd09ec42e..affd1c9df4882 100644 --- a/compiler/rustc_session/src/cstore.rs +++ b/compiler/rustc_session/src/cstore.rs @@ -4,12 +4,10 @@ use crate::search_paths::PathKind; use crate::utils::NativeLibKind; -use crate::Session; 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::definitions::{DefKey, DefPath, DefPathHash, Definitions}; -use rustc_span::hygiene::{ExpnHash, ExpnId}; use rustc_span::symbol::Symbol; use rustc_span::Span; use rustc_target::spec::abi::Abi; @@ -223,13 +221,6 @@ pub trait CrateStore: std::fmt::Debug { /// Fetch a DefId from a DefPathHash for a foreign crate. fn def_path_hash_to_def_id(&self, cnum: CrateNum, hash: DefPathHash) -> DefId; - fn expn_hash_to_expn_id( - &self, - sess: &Session, - cnum: CrateNum, - index_guess: u32, - hash: ExpnHash, - ) -> ExpnId; } pub type CrateStoreDyn = dyn CrateStore + sync::DynSync + sync::DynSend; From 24fc6e96d6acd8ee75023f1a91893df441911fc3 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 26 Mar 2024 15:53:05 +0000 Subject: [PATCH 3/3] Make `def_path_hash_to_def_id` a hook --- .../src/rmeta/decoder/cstore_impl.rs | 14 +++++++++----- compiler/rustc_middle/src/hooks/mod.rs | 8 ++++++++ compiler/rustc_middle/src/ty/context.rs | 6 +----- compiler/rustc_session/src/cstore.rs | 3 --- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 1720a4642c599..2935d5b8f6328 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -650,14 +650,18 @@ impl CrateStore for CStore { fn def_path_hash(&self, def: DefId) -> DefPathHash { self.get_crate_data(def.krate).def_path_hash(def.index) } - - fn def_path_hash_to_def_id(&self, cnum: CrateNum, hash: DefPathHash) -> DefId { - let def_index = self.get_crate_data(cnum).def_path_hash_to_def_index(hash); - DefId { krate: cnum, index: def_index } - } } fn provide_cstore_hooks(providers: &mut Providers) { + providers.hooks.def_path_hash_to_def_id_extern = |tcx, hash, stable_crate_id| { + // 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 def_index = cstore.get_crate_data(cnum).def_path_hash_to_def_index(hash); + DefId { krate: cnum, index: def_index } + }; + providers.hooks.expn_hash_to_expn_id = |tcx, cnum, index_guess, hash| { let cstore = CStore::from_tcx(tcx.tcx); cstore.get_crate_data(cnum).expn_hash_to_expn_id(tcx.sess, index_guess, hash) diff --git a/compiler/rustc_middle/src/hooks/mod.rs b/compiler/rustc_middle/src/hooks/mod.rs index b0f08334c542f..f7ce15d0a8dcb 100644 --- a/compiler/rustc_middle/src/hooks/mod.rs +++ b/compiler/rustc_middle/src/hooks/mod.rs @@ -6,6 +6,8 @@ use crate::mir; use crate::query::TyCtxtAt; use crate::ty::{Ty, TyCtxt}; +use rustc_hir::def_id::{DefId, DefPathHash}; +use rustc_session::StableCrateId; use rustc_span::def_id::{CrateNum, LocalDefId}; use rustc_span::{ExpnHash, ExpnId, DUMMY_SP}; @@ -94,4 +96,10 @@ declare_hooks! { index_guess: u32, hash: ExpnHash ) -> ExpnId; + + /// Converts a `DefPathHash` to its corresponding `DefId` in the current compilation + /// session, if it still exists. This is used during incremental compilation to + /// turn a deserialized `DefPathHash` into its current `DefId`. + /// Will fetch a DefId from a DefPathHash for a foreign crate. + hook def_path_hash_to_def_id_extern(hash: DefPathHash, stable_crate_id: StableCrateId) -> DefId; } diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 3393f44484388..b5c49be70df16 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -1073,11 +1073,7 @@ impl<'tcx> TyCtxt<'tcx> { if stable_crate_id == self.stable_crate_id(LOCAL_CRATE) { self.untracked.definitions.read().local_def_path_hash_to_def_id(hash, err).to_def_id() } else { - // If this is a DefPathHash from an upstream crate, let the CrateStore map - // it to a DefId. - 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) + self.def_path_hash_to_def_id_extern(hash, stable_crate_id) } } diff --git a/compiler/rustc_session/src/cstore.rs b/compiler/rustc_session/src/cstore.rs index affd1c9df4882..cb6656bae062c 100644 --- a/compiler/rustc_session/src/cstore.rs +++ b/compiler/rustc_session/src/cstore.rs @@ -218,9 +218,6 @@ pub trait CrateStore: std::fmt::Debug { 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; - - /// Fetch a DefId from a DefPathHash for a foreign crate. - fn def_path_hash_to_def_id(&self, cnum: CrateNum, hash: DefPathHash) -> DefId; } pub type CrateStoreDyn = dyn CrateStore + sync::DynSync + sync::DynSend;