diff --git a/compiler/rustc_hir/src/definitions.rs b/compiler/rustc_hir/src/definitions.rs index e9801a8a5231b..55f739ba38a5f 100644 --- a/compiler/rustc_hir/src/definitions.rs +++ b/compiler/rustc_hir/src/definitions.rs @@ -114,6 +114,28 @@ impl PerParentDisambiguatorState { next: Default::default(), } } + + /// If there are multiple definitions with the same DefPathData and the same parent, use + /// `self` to differentiate them. Distinct `PerParentDisambiguatorState` instances are not + /// guaranteed to generate unique disambiguators and should instead ensure that the `parent` + /// and `data` pair is distinct from other instances. + pub fn disambiguate( + &mut self, + _parent: LocalDefId, + data: DefPathData, + ) -> DisambiguatedDefPathData { + #[cfg(debug_assertions)] + debug_assert_eq!( + _parent, + self.parent.expect("must be set"), + "provided parent and parent in disambiguator must be the same" + ); + + let next_disamb = self.next.entry(data).or_insert(0); + let disambiguator = *next_disamb; + *next_disamb = next_disamb.checked_add(1).expect("disambiguator overflow"); + DisambiguatedDefPathData { disambiguator, data } + } } #[extension(pub trait PerParentDisambiguatorsMap)] @@ -144,16 +166,35 @@ pub struct DefKey { } impl DefKey { - pub(crate) fn compute_stable_hash(&self, parent: DefPathHash) -> DefPathHash { + #[inline] + pub fn get_opt_name(&self) -> Option { + self.disambiguated_data.data.get_opt_name() + } +} + +/// A pair of `DefPathData` and an integer disambiguator. The integer is +/// normally `0`, but in the event that there are multiple defs with the +/// same `parent` and `data`, we use this field to disambiguate +/// between them. This introduces some artificial ordering dependency +/// but means that if you have, e.g., two impls for the same type in +/// the same module, they do get distinct `DefId`s. +#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, Encodable, BlobDecodable)] +pub struct DisambiguatedDefPathData { + pub data: DefPathData, + pub disambiguator: u32, +} + +impl DisambiguatedDefPathData { + pub fn compute_stable_hash(self, parent: DefPathHash) -> DefPathHash { let mut hasher = StableHasher::new(); // The new path is in the same crate as `parent`, and will contain the stable_crate_id. // Therefore, we only need to include information of the parent's local hash. parent.local_hash().hash(&mut hasher); - let DisambiguatedDefPathData { ref data, disambiguator } = self.disambiguated_data; + let DisambiguatedDefPathData { data, disambiguator } = self; - std::mem::discriminant(data).hash(&mut hasher); + std::mem::discriminant(&data).hash(&mut hasher); if let Some(name) = data.hashed_symbol() { // Get a stable hash by considering the symbol chars rather than // the symbol index. @@ -171,25 +212,6 @@ impl DefKey { DefPathHash::new(parent.stable_crate_id(), local_hash) } - #[inline] - pub fn get_opt_name(&self) -> Option { - self.disambiguated_data.data.get_opt_name() - } -} - -/// A pair of `DefPathData` and an integer disambiguator. The integer is -/// normally `0`, but in the event that there are multiple defs with the -/// same `parent` and `data`, we use this field to disambiguate -/// between them. This introduces some artificial ordering dependency -/// but means that if you have, e.g., two impls for the same type in -/// the same module, they do get distinct `DefId`s. -#[derive(Copy, Clone, PartialEq, Debug, Encodable, BlobDecodable)] -pub struct DisambiguatedDefPathData { - pub data: DefPathData, - pub disambiguator: u32, -} - -impl DisambiguatedDefPathData { pub fn as_sym(&self, verbose: bool) -> Symbol { match self.data.name() { DefPathDataName::Named(name) => { @@ -384,16 +406,7 @@ impl Definitions { } /// Creates a definition with a parent definition. - /// If there are multiple definitions with the same DefPathData and the same parent, use - /// `disambiguator` to differentiate them. Distinct `DisambiguatorState` instances are not - /// guaranteed to generate unique disambiguators and should instead ensure that the `parent` - /// and `data` pair is distinct from other instances. - pub fn create_def( - &mut self, - parent: LocalDefId, - data: DefPathData, - disambiguator: &mut PerParentDisambiguatorState, - ) -> LocalDefId { + pub fn create_def(&mut self, parent: LocalDefId, data: DisambiguatedDefPathData) -> LocalDefId { // We can't use `Debug` implementation for `LocalDefId` here, since it tries to acquire a // reference to `Definitions` and we're already holding a mutable reference. debug!( @@ -402,30 +415,12 @@ impl Definitions { ); // The root node must be created in `new()`. - assert!(data != DefPathData::CrateRoot); - - // Find the next free disambiguator for this key. - let disambiguator = { - #[cfg(debug_assertions)] - debug_assert_eq!( - parent, - disambiguator.parent.expect("must be set"), - "provided parent and parent in disambiguator must be the same" - ); - - let next_disamb = disambiguator.next.entry(data).or_insert(0); - let disambiguator = *next_disamb; - *next_disamb = next_disamb.checked_add(1).expect("disambiguator overflow"); - disambiguator - }; - let key = DefKey { - parent: Some(parent.local_def_index), - disambiguated_data: DisambiguatedDefPathData { data, disambiguator }, - }; + assert!(data.data != DefPathData::CrateRoot); let parent_hash = self.table.def_path_hash(parent.local_def_index); - let def_path_hash = key.compute_stable_hash(parent_hash); + let def_path_hash = data.compute_stable_hash(parent_hash); + let key = DefKey { parent: Some(parent.local_def_index), disambiguated_data: data }; debug!("create_def: after disambiguation, key = {:?}", key); // Create the definition. diff --git a/compiler/rustc_hir/src/tests.rs b/compiler/rustc_hir/src/tests.rs index 3b797c1f1038a..2aa5f7fd33bad 100644 --- a/compiler/rustc_hir/src/tests.rs +++ b/compiler/rustc_hir/src/tests.rs @@ -31,15 +31,10 @@ fn def_path_hash_depends_on_crate_id() { let parent_hash = DefPathHash::new(stable_crate_id, Hash64::new(stable_crate_id.as_u64())); - let key = DefKey { - parent: None, - disambiguated_data: DisambiguatedDefPathData { - data: DefPathData::CrateRoot, - disambiguator: 0, - }, - }; - - key.compute_stable_hash(parent_hash) + let disambiguated_data = + DisambiguatedDefPathData { data: DefPathData::CrateRoot, disambiguator: 0 }; + + disambiguated_data.compute_stable_hash(parent_hash) } }) } diff --git a/compiler/rustc_interface/src/callbacks.rs b/compiler/rustc_interface/src/callbacks.rs index b2ad70bc4811b..fc54c871c3ed7 100644 --- a/compiler/rustc_interface/src/callbacks.rs +++ b/compiler/rustc_interface/src/callbacks.rs @@ -23,7 +23,10 @@ fn track_span_parent(def_id: rustc_span::def_id::LocalDefId) { // Skip doing anything if we aren't tracking dependencies. let tracks_deps = match icx.task_deps { TaskDepsRef::Allow(..) => true, - TaskDepsRef::EvalAlways | TaskDepsRef::Ignore | TaskDepsRef::Forbid => false, + TaskDepsRef::EvalAlways + | TaskDepsRef::Ignore + | TaskDepsRef::Forbid + | TaskDepsRef::Replay => false, }; if tracks_deps { let _span = icx.tcx.source_span(def_id); diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index cb41974af41b0..e90e67a943300 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -11,9 +11,7 @@ use rustc_codegen_ssa::traits::CodegenBackend; use rustc_codegen_ssa::{CompiledModules, CrateInfo}; use rustc_data_structures::indexmap::IndexMap; use rustc_data_structures::steal::Steal; -use rustc_data_structures::sync::{ - AppendOnlyIndexVec, DynSend, DynSync, FreezeLock, WorkerLocal, par_fns, -}; +use rustc_data_structures::sync::{DynSend, DynSync, WorkerLocal, par_fns}; use rustc_data_structures::thousands; use rustc_errors::timings::TimingSection; use rustc_errors::{Diag, DiagCtxtHandle, Diagnostic, Level}; @@ -21,8 +19,7 @@ use rustc_expand::base::{ExtCtxt, LintStoreExpand}; use rustc_feature::Features; use rustc_fs_util::try_canonicalize; use rustc_hir::attrs::AttributeKind; -use rustc_hir::def_id::{LOCAL_CRATE, StableCrateId, StableCrateIdMap}; -use rustc_hir::definitions::Definitions; +use rustc_hir::def_id::{LOCAL_CRATE, StableCrateId}; use rustc_hir::limit::Limit; use rustc_hir::{Attribute, MaybeOwner, Target, find_attr}; use rustc_incremental::setup_dep_graph; @@ -38,7 +35,6 @@ use rustc_passes::{abi_test, input_stats, layout_test}; use rustc_resolve::{Resolver, ResolverOutputs}; use rustc_session::Session; use rustc_session::config::{CrateType, Input, OutFileName, OutputFilenames, OutputType}; -use rustc_session::cstore::Untracked; use rustc_session::errors::feature_err; use rustc_session::output::{filename_for_input, invalid_output_for_target}; use rustc_session::search_paths::PathKind; @@ -943,13 +939,7 @@ pub fn create_and_enter_global_ctxt FnOnce(TyCtxt<'tcx>) -> T>( let dep_graph = setup_dep_graph(sess, crate_name, stable_crate_id); - let cstore = - FreezeLock::new(Box::new(CStore::new(compiler.codegen_backend.metadata_loader())) as _); - let definitions = FreezeLock::new(Definitions::new(stable_crate_id)); - - let stable_crate_ids = FreezeLock::new(StableCrateIdMap::default()); - let untracked = - Untracked { cstore, source_span: AppendOnlyIndexVec::new(), definitions, stable_crate_ids }; + let cstore = Box::new(CStore::new(compiler.codegen_backend.metadata_loader())) as _; // We're constructing the HIR here; we don't care what we will // read, since we haven't even constructed the *input* to @@ -990,7 +980,7 @@ pub fn create_and_enter_global_ctxt FnOnce(TyCtxt<'tcx>) -> T>( stable_crate_id, &arena, &hir_arena, - untracked, + cstore, dep_graph, rustc_query_impl::make_dep_kind_vtables(&arena), rustc_query_impl::query_system( diff --git a/compiler/rustc_middle/src/dep_graph/graph.rs b/compiler/rustc_middle/src/dep_graph/graph.rs index a219809541cc1..7b5d12791a60e 100644 --- a/compiler/rustc_middle/src/dep_graph/graph.rs +++ b/compiler/rustc_middle/src/dep_graph/graph.rs @@ -12,11 +12,13 @@ use rustc_data_structures::stable_hasher::{StableHash, StableHasher}; use rustc_data_structures::sync::{AtomicU64, Lock}; use rustc_data_structures::unord::UnordMap; use rustc_errors::DiagInner; +use rustc_hir::definitions::DisambiguatedDefPathData; use rustc_index::IndexVec; use rustc_macros::{Decodable, Encodable}; use rustc_serialize::opaque::{FileEncodeResult, FileEncoder}; use rustc_session::Session; use rustc_span::Symbol; +use rustc_span::def_id::LocalDefId; use tracing::instrument; #[cfg(debug_assertions)] use {super::debug::EdgeFilter, std::env}; @@ -50,6 +52,8 @@ pub enum QuerySideEffect { /// if we mark the query as green, as that query will have /// the side effect dep node as a dependency. CheckFeature { symbol: Symbol }, + /// Creates a new definition. + CreateDef { parent: LocalDefId, data: DisambiguatedDefPathData }, } #[derive(Clone)] @@ -220,6 +224,13 @@ impl DepGraph { with_deps(TaskDepsRef::Ignore, op) } + pub fn with_replay(&self, op: OP) -> R + where + OP: FnOnce() -> R, + { + with_deps(TaskDepsRef::Replay, op) + } + /// Used to wrap the deserialization of a query result from disk, /// This method enforces that no new `DepNodes` are created during /// query result deserialization. @@ -458,7 +469,7 @@ impl DepGraph { // queries. They are re-evaluated unconditionally anyway. return; } - TaskDepsRef::Ignore => return, + TaskDepsRef::Ignore | TaskDepsRef::Replay => return, TaskDepsRef::Forbid => { // Reading is forbidden in this context. ICE with a useful error message. panic_on_forbidden_read(data, dep_node_index) @@ -508,7 +519,7 @@ impl DepGraph { pub fn record_diagnostic<'tcx>(&self, tcx: TyCtxt<'tcx>, diagnostic: &DiagInner) { if let Some(ref data) = self.data { read_deps(|task_deps| match task_deps { - TaskDepsRef::EvalAlways | TaskDepsRef::Ignore => return, + TaskDepsRef::EvalAlways | TaskDepsRef::Ignore | TaskDepsRef::Replay => return, TaskDepsRef::Forbid | TaskDepsRef::Allow(..) => { let dep_node_index = data .encode_side_effect(tcx, QuerySideEffect::Diagnostic(diagnostic.clone())); @@ -533,7 +544,9 @@ impl DepGraph { side_effect: QuerySideEffect, ) -> DepNodeIndex { if let Some(ref data) = self.data { - data.encode_side_effect(tcx, side_effect) + let dep_node_index = data.encode_side_effect(tcx, side_effect); + self.read_index(dep_node_index); + dep_node_index } else { self.next_virtual_depnode_index() } @@ -600,7 +613,7 @@ impl DepGraph { TaskDepsRef::EvalAlways => { edges.push(DepNodeIndex::FOREVER_RED_NODE); } - TaskDepsRef::Ignore => {} + TaskDepsRef::Ignore | TaskDepsRef::Replay => {} TaskDepsRef::Forbid => { panic!("Cannot summarize when dependencies are not recorded.") } @@ -725,6 +738,9 @@ impl DepGraphData { QuerySideEffect::CheckFeature { symbol } => { tcx.sess.used_features.lock().insert(*symbol, dep_node_index.as_u32()); } + QuerySideEffect::CreateDef { parent, data } => { + tcx.untracked().definitions.write().create_def(*parent, *data); + } } // This will just overwrite the same value for concurrent calls. @@ -1211,6 +1227,9 @@ pub enum TaskDepsRef<'a> { EvalAlways, /// New dependencies are ignored. This is also used for `dep_graph.with_ignore`. Ignore, + /// This query has already been marked green, its dependency graph is recorded, + /// but we need to re-run the code to get its result. + Replay, /// Any attempt to add new dependencies will cause a panic. /// This is used when decoding a query result from disk, /// to ensure that the decoding process doesn't itself diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs index a6ff238ad6f0b..2f54b8bcd9fa8 100644 --- a/compiler/rustc_middle/src/query/erase.rs +++ b/compiler/rustc_middle/src/query/erase.rs @@ -214,6 +214,7 @@ impl_erasable_for_types_with_no_type_params! { rustc_hir::OpaqueTyOrigin, rustc_hir::def::DefKind, rustc_hir::def_id::DefId, + rustc_hir::def_id::LocalDefId, rustc_middle::middle::codegen_fn_attrs::SanitizerFnAttrs, rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault, rustc_middle::mir::ConstQualifs, diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 6c68a83f9357a..22e18fe04bbd1 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -26,11 +26,12 @@ use rustc_data_structures::sharded::{IntoPointer, ShardedHashMap}; use rustc_data_structures::stable_hasher::StableHash; use rustc_data_structures::steal::Steal; use rustc_data_structures::sync::{ - self, DynSend, DynSync, FreezeReadGuard, Lock, RwLock, WorkerLocal, + self, AppendOnlyIndexVec, DynSend, DynSync, FreezeLock, FreezeReadGuard, Lock, RwLock, + WorkerLocal, }; use rustc_errors::{Applicability, Diag, DiagCtxtHandle, Diagnostic, MultiSpan}; use rustc_hir::def::DefKind; -use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId}; +use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId, StableCrateIdMap}; use rustc_hir::definitions::{DefPathData, Definitions, PerParentDisambiguatorState}; use rustc_hir::intravisit::VisitorExt; use rustc_hir::lang_items::LangItem; @@ -51,7 +52,7 @@ use tracing::{debug, instrument}; use crate::arena::Arena; use crate::dep_graph::dep_node::make_metadata; -use crate::dep_graph::{DepGraph, DepKindVTable, DepNodeIndex}; +use crate::dep_graph::{DepGraph, DepKindVTable, QuerySideEffect, TaskDepsRef}; use crate::ich::StableHashingContext; use crate::infer::canonical::{CanonicalParamEnvCache, CanonicalVarKind}; use crate::lint::emit_lint_base; @@ -938,7 +939,7 @@ impl<'tcx> TyCtxt<'tcx> { stable_crate_id: StableCrateId, arena: &'tcx WorkerLocal>, hir_arena: &'tcx WorkerLocal>, - untracked: Untracked, + cstore: Box, dep_graph: DepGraph, dep_kind_vtables: &'tcx [DepKindVTable<'tcx>], query_system: QuerySystem<'tcx>, @@ -947,6 +948,17 @@ impl<'tcx> TyCtxt<'tcx> { jobserver_proxy: Arc, f: impl FnOnce(TyCtxt<'tcx>) -> T, ) -> T { + let cstore = FreezeLock::new(cstore); + let definitions = FreezeLock::new(Definitions::new(stable_crate_id)); + + let stable_crate_ids = FreezeLock::new(StableCrateIdMap::default()); + let untracked = Untracked { + cstore, + source_span: AppendOnlyIndexVec::new(), + definitions, + stable_crate_ids, + }; + let data_layout = sess.target.parse_data_layout().unwrap_or_else(|err| { sess.dcx().emit_fatal(err); }); @@ -1300,25 +1312,39 @@ impl<'tcx> TyCtxt<'tcx> { disambiguator: &mut PerParentDisambiguatorState, ) -> TyCtxtFeed<'tcx, LocalDefId> { let data = override_def_path_data.unwrap_or_else(|| def_kind.def_path_data(name)); - // The following call has the side effect of modifying the tables inside `definitions`. - // These very tables are relied on by the incr. comp. engine to decode DepNodes and to - // decode the on-disk cache. - // - // Any LocalDefId which is used within queries, either as key or result, either: - // - has been created before the construction of the TyCtxt; - // - has been created by this call to `create_def`. - // As a consequence, this LocalDefId is always re-created before it is needed by the incr. - // comp. engine itself. - let def_id = self.untracked.definitions.write().create_def(parent, data, disambiguator); - - // This function modifies `self.definitions` using a side-effect. - // We need to ensure that these side effects are re-run by the incr. comp. engine. - // Depending on the forever-red node will tell the graph that the calling query - // needs to be re-evaluated. - self.dep_graph.read_index(DepNodeIndex::FOREVER_RED_NODE); + + // Find the next free disambiguator for this key. + let data = disambiguator.disambiguate(parent, data); + + let def_id = ty::tls::with_context(|icx| match icx.task_deps { + // If we are not tracking dependencies, we can use global mutable state. + // This is only an optimization to avoid the cost of registering the dep-node. + TaskDepsRef::EvalAlways | TaskDepsRef::Forbid | TaskDepsRef::Ignore => { + self.untracked.definitions.write().create_def(parent, data) + } + + // We are tracking dependencies, so we need to record a side-effect for the dep-graph + // to pick up in next execution. + TaskDepsRef::Allow(..) => { + self.dep_graph + .encode_side_effect(self, QuerySideEffect::CreateDef { parent, data }); + self.untracked.definitions.write().create_def(parent, data) + } + + // We are in replay mode: the def-id has already been created by + // `dep_graph.force_side_effect`. We need to recover it, without creating a new one. + TaskDepsRef::Replay => { + let parent_hash = self.def_path_hash(parent.to_def_id()); + let def_path_hash = data.compute_stable_hash(parent_hash); + self.def_path_hash_to_def_id(def_path_hash) + .expect("first execution should have created this definition") + .expect_local() + } + }); let feed = TyCtxtFeed { tcx: self, key: def_id }; feed.def_kind(def_kind); + // Unique types created for closures participate in type privacy checking. // They have visibilities inherited from the module they are defined in. // Visibilities for opaque types are meaningless, but still provided diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index ed9ad8c7a0a68..8ef937948e3dc 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -529,7 +529,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>( // We could not load a result from the on-disk cache, so recompute. The dep-graph for // this computation is already in-place, so we can just call the query provider. let prof_timer = tcx.prof.query_provider(); - let value = tcx.dep_graph.with_ignore(|| (query.invoke_provider_fn)(tcx, key)); + let value = tcx.dep_graph.with_replay(|| (query.invoke_provider_fn)(tcx, key)); prof_timer.finish_with_query_invocation_id(dep_node_index.into()); (value, true)