Also store the DefPathHash as part of a DefId creation side effect, so we can reliably recreate the same DefId over and over again

Recreate `DefId`s when a cached query gets replayed

Author: oli-obk

compiler/rustc_hir/src/definitions.rs

@@ -67,7 +67,8 @@ impl DefPathTable {
             // being used.
             //
             // See the documentation for DefPathHash for more information.
-            panic!(
+            assert_eq!(
+                def_path1, def_path2,
                 "found DefPathHash collision between {def_path1:#?} and {def_path2:#?}. \
                     Compilation cannot continue."
             );
@@ -214,7 +215,7 @@ impl fmt::Display for DisambiguatedDefPathData {
     }
 }
 
-#[derive(Clone, Debug, Encodable, Decodable)]
+#[derive(Clone, Debug, Encodable, Decodable, PartialEq)]
 pub struct DefPath {
     /// The path leading from the crate root to the item.
     pub data: Vec<DisambiguatedDefPathData>,
@@ -389,7 +390,7 @@ impl Definitions {
         parent: LocalDefId,
         data: DefPathData,
         disambiguator: u32,
-    ) -> LocalDefId {
+    ) -> (LocalDefId, DefPathHash) {
         // 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!(
@@ -411,7 +412,7 @@ impl Definitions {
         debug!("create_def: after disambiguation, key = {:?}", key);
 
         // Create the definition.
-        LocalDefId { local_def_index: self.table.allocate(key, def_path_hash) }
+        (LocalDefId { local_def_index: self.table.allocate(key, def_path_hash) }, def_path_hash)
     }
 
     #[inline(always)]

compiler/rustc_interface/src/callbacks.rs

@@ -14,9 +14,11 @@ use std::fmt;
 use rustc_errors::{DiagInner, TRACK_DIAGNOSTIC};
 use rustc_middle::dep_graph::{DepNodeExt, TaskDepsRef};
 use rustc_middle::ty::tls;
+use rustc_middle::util::Providers;
 use rustc_query_impl::QueryCtxt;
 use rustc_query_system::dep_graph::dep_node::default_dep_kind_debug;
 use rustc_query_system::dep_graph::{DepContext, DepKind, DepNode};
+use rustc_query_system::query::DefIdInfo;
 
 fn track_span_parent(def_id: rustc_span::def_id::LocalDefId) {
     tls::with_context_opt(|icx| {
@@ -113,3 +115,14 @@ pub fn setup_callbacks() {
         .swap(&(dep_node_debug as fn(_, &mut fmt::Formatter<'_>) -> _));
     TRACK_DIAGNOSTIC.swap(&(track_diagnostic as _));
 }
+
+pub fn provide(providers: &mut Providers) {
+    providers.create_def_raw = |tcx, (parent, data, disambiguator)| {
+        let (def_id, hash) =
+            tcx.untracked().definitions.write().create_def(parent, data, disambiguator);
+
+        tcx.dep_graph
+            .record_def(QueryCtxt::new(tcx), DefIdInfo { parent, data, hash, disambiguator });
+        def_id
+    }
+}

compiler/rustc_interface/src/passes.rs

@@ -799,6 +799,7 @@ pub static DEFAULT_QUERY_PROVIDERS: LazyLock<Providers> = LazyLock::new(|| {
     rustc_lint::provide(providers);
     rustc_symbol_mangling::provide(providers);
     rustc_codegen_ssa::provide(providers);
+    crate::callbacks::provide(providers);
     *providers
 });
 

compiler/rustc_middle/src/dep_graph/mod.rs

@@ -1,5 +1,6 @@
 use rustc_data_structures::profiling::SelfProfilerRef;
 use rustc_query_system::ich::StableHashingContext;
+use rustc_query_system::query::DefIdInfo;
 use rustc_session::Session;
 
 use crate::ty::{self, TyCtxt};
@@ -80,6 +81,11 @@ impl<'tcx> DepContext for TyCtxt<'tcx> {
         self.sess
     }
 
+    fn create_def(&self, &DefIdInfo { parent, data, disambiguator: index, hash }: &DefIdInfo) {
+        let (_, h) = self.untracked().definitions.write().create_def(parent, data, index);
+        assert_eq!(hash, h);
+    }
+
     #[inline]
     fn dep_kind_info(&self, dk: DepKind) -> &DepKindStruct<'tcx> {
         &self.query_kinds[dk.as_usize()]

compiler/rustc_middle/src/hir/mod.rs

@@ -239,7 +239,4 @@ pub fn provide(providers: &mut Providers) {
     providers.in_scope_traits_map = |tcx, id| {
         tcx.hir_crate(()).owners[id.def_id].as_owner().map(|owner_info| &owner_info.trait_map)
     };
-    providers.create_def_raw = |tcx, (parent, data, disambiguator)| {
-        tcx.untracked().definitions.write().create_def(parent, data, disambiguator)
-    }
 }

compiler/rustc_middle/src/query/mod.rs

@@ -2059,6 +2059,7 @@ rustc_queries! {
         desc { |tcx| "computing visibility of `{}`", tcx.def_path_str(def_id) }
         separate_provide_extern
         feedable
+        cache_on_disk_if { def_id.is_local() }
     }
 
     query inhabited_predicate_adt(key: DefId) -> ty::inhabitedness::InhabitedPredicate<'tcx> {

compiler/rustc_middle/src/ty/context.rs

@@ -1990,6 +1990,7 @@ impl<'tcx> TyCtxtAt<'tcx> {
 
 impl<'tcx> TyCtxt<'tcx> {
     /// `tcx`-dependent operations performed for every created definition.
+    #[instrument(level = "trace", skip(self))]
     pub fn create_def(
         self,
         parent: LocalDefId,
@@ -2009,13 +2010,7 @@ impl<'tcx> TyCtxt<'tcx> {
                     // 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.
-                    self.untracked.definitions.write().create_def(parent, data, disambiguator.next(parent, data))
+                    self.untracked.definitions.write().create_def(parent, data, disambiguator.next(parent, data)).0
                 }
                 TaskDepsRef::Forbid => bug!(
                     "cannot create definition {parent:?} {data:?} without being able to register task dependencies"

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -26,7 +26,7 @@ use super::serialized::{GraphEncoder, SerializedDepGraph, SerializedDepNodeIndex
 use super::{DepContext, DepKind, DepNode, Deps, HasDepContext, WorkProductId};
 use crate::dep_graph::edges::EdgesVec;
 use crate::ich::StableHashingContext;
-use crate::query::{QueryContext, QuerySideEffect};
+use crate::query::{DefIdInfo, QueryContext, QuerySideEffect};
 
 #[derive(Clone)]
 pub struct DepGraph<D: Deps> {
@@ -260,6 +260,7 @@ impl<D: Deps> DepGraph<D> {
     }
 
     #[inline(always)]
+    /// A helper for `codegen_cranelift`.
     pub fn with_task<Ctxt: HasDepContext<Deps = D>, A: Debug, R>(
         &self,
         key: DepNode,
@@ -530,6 +531,18 @@ impl<D: Deps> DepGraph<D> {
             })
         }
     }
+
+    pub fn record_def<Qcx: QueryContext>(&self, qcx: Qcx, def: DefIdInfo) {
+        if let Some(ref data) = self.data {
+            D::read_deps(|task_deps| match task_deps {
+                TaskDepsRef::EvalAlways | TaskDepsRef::Ignore => return,
+                TaskDepsRef::Forbid | TaskDepsRef::Allow(..) => {
+                    self.read_index(data.encode_side_effect(qcx, QuerySideEffect::Definition(def)));
+                }
+            })
+        }
+    }
+
     /// This forces a diagnostic node green by running its side effect. `prev_index` would
     /// refer to a node created used [Self::record_diagnostic] in the previous session.
     #[inline]
@@ -667,7 +680,7 @@ impl<D: Deps> DepGraphData<D> {
         self.debug_loaded_from_disk.lock().insert(dep_node);
     }
 
-    /// This encodes a diagnostic by creating a node with an unique index and assoicating
+    /// This encodes a diagnostic by creating a node with an unique index and associating
     /// `diagnostic` with it, for use in the next session.
     #[inline]
     fn encode_side_effect<Qcx: QueryContext>(
@@ -705,6 +718,9 @@ impl<D: Deps> DepGraphData<D> {
                 QuerySideEffect::Diagnostic(diagnostic) => {
                     qcx.dep_context().sess().dcx().emit_diagnostic(diagnostic.clone());
                 }
+                QuerySideEffect::Definition(def_id_info) => {
+                    qcx.dep_context().create_def(def_id_info)
+                }
             }
 
             // Use `send_and_color` as `promote_node_and_deps_to_current` expects all

compiler/rustc_query_system/src/dep_graph/mod.rs

@@ -19,6 +19,7 @@ use tracing::instrument;
 
 use self::graph::{MarkFrame, print_markframe_trace};
 use crate::ich::StableHashingContext;
+use crate::query::DefIdInfo;
 
 pub trait DepContext: Copy {
     type Deps: Deps;
@@ -35,6 +36,8 @@ pub trait DepContext: Copy {
     /// Access the compiler session.
     fn sess(&self) -> &Session;
 
+    fn create_def(&self, def: &DefIdInfo);
+
     fn dep_kind_info(&self, dep_node: DepKind) -> &DepKindStruct<Self>;
 
     #[inline(always)]

compiler/rustc_query_system/src/query/mod.rs

@@ -21,9 +21,10 @@ use rustc_data_structures::sync::{DynSend, DynSync};
 use rustc_errors::DiagInner;
 use rustc_hashes::Hash64;
 use rustc_hir::def::DefKind;
+use rustc_hir::def_id::DefPathHash;
 use rustc_macros::{Decodable, Encodable};
 use rustc_span::Span;
-use rustc_span::def_id::DefId;
+use rustc_span::def_id::{DefId, LocalDefId};
 
 pub use self::config::{HashResult, QueryConfig};
 use crate::dep_graph::{DepKind, DepNodeIndex, HasDepContext, SerializedDepNodeIndex};
@@ -147,6 +148,19 @@ pub enum QuerySideEffect {
     /// the query as green, as that query will have the side
     /// effect dep node as a dependency.
     Diagnostic(DiagInner),
+
+    /// A `DefId` that was created during query execution.
+    /// These `DefId`s will be re-created when we mark the query as green.
+    Definition(DefIdInfo),
+}
+
+#[derive(Debug, Clone, Encodable, Decodable, PartialEq)]
+pub struct DefIdInfo {
+    pub parent: LocalDefId,
+    pub data: rustc_hir::definitions::DefPathData,
+    pub hash: DefPathHash,
+    /// Disambiguator
+    pub disambiguator: u32,
 }
 
 pub trait QueryContext: HasDepContext {