Make create_def a query.

Author: cjgillot

compiler/rustc_hir/src/definitions.rs

@@ -11,7 +11,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::StableHasher;
 use rustc_hashes::Hash64;
 use rustc_index::IndexVec;
-use rustc_macros::{BlobDecodable, Decodable, Encodable, extension};
+use rustc_macros::{BlobDecodable, Decodable, Encodable, HashStable, extension};
 use rustc_span::def_id::LocalDefIdMap;
 use rustc_span::{Symbol, kw, sym};
 use tracing::{debug, instrument};
@@ -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)]
@@ -183,7 +205,7 @@ impl DefKey {
 /// 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)]
+#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, Encodable, BlobDecodable, HashStable)]
 pub struct DisambiguatedDefPathData {
     pub data: DefPathData,
     pub disambiguator: u32,
@@ -277,7 +299,7 @@ impl DefPath {
 }
 
 /// New variants should only be added in synchronization with `enum DefKind`.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Encodable, BlobDecodable)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Encodable, BlobDecodable, HashStable)]
 pub enum DefPathData {
     // Root: these should only be used for the root nodes, because
     // they are treated specially by the `def_path` function.
@@ -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,26 +415,9 @@ 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"
-            );
+        assert!(data.data != DefPathData::CrateRoot);
 
-            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 },
-        };
+        let key = DefKey { parent: Some(parent.local_def_index), disambiguated_data: data };
 
         let parent_hash = self.table.def_path_hash(parent.local_def_index);
         let def_path_hash = key.compute_stable_hash(parent_hash);

compiler/rustc_middle/src/queries.rs

@@ -67,6 +67,7 @@ use rustc_hir::def::{DefKind, DocLinkResMap};
 use rustc_hir::def_id::{
     CrateNum, DefId, DefIdMap, LocalDefId, LocalDefIdMap, LocalDefIdSet, LocalModDefId,
 };
+use rustc_hir::definitions::DisambiguatedDefPathData;
 use rustc_hir::lang_items::{LangItem, LanguageItems};
 use rustc_hir::{ItemLocalId, ItemLocalMap, PreciseCapturingArgKind, TraitCandidate};
 use rustc_index::IndexVec;
@@ -198,6 +199,17 @@ rustc_queries! {
         desc { "getting the source span" }
     }
 
+    /// Create a new definition.
+    ///
+    /// This query is meant to wrap a side-effect and return the resulting newly created
+    /// definition. In incremental mode, when replaying a query, this query caches the side-effect
+    /// to avoid performing it twice.
+    query create_def_raw(key: (LocalDefId, DisambiguatedDefPathData)) -> LocalDefId {
+        // Accesses untracked data
+        eval_always
+        desc { "create a new definition for `{}::{:?}`", tcx.def_path_str(key.0), key.1 }
+    }
+
     /// Represents crate as a whole (as distinct from the top-level crate module).
     ///
     /// If you call `tcx.hir_crate(())` we will have to assume that any change

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_id::DefId>,
     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,

compiler/rustc_middle/src/query/keys.rs

@@ -7,6 +7,7 @@ use std::hash::Hash;
 use rustc_ast::tokenstream::TokenStream;
 use rustc_data_structures::stable_hasher::HashStable;
 use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId, LocalModDefId};
+use rustc_hir::definitions::DisambiguatedDefPathData;
 use rustc_hir::hir_id::OwnerId;
 use rustc_span::{DUMMY_SP, Ident, LocalExpnId, Span, Symbol};
 
@@ -360,3 +361,11 @@ impl<'tcx> QueryKey for (ty::Instance<'tcx>, CollectionMode) {
         self.0.default_span(tcx)
     }
 }
+
+impl QueryKey for (LocalDefId, DisambiguatedDefPathData) {
+    type Cache<V> = DefaultCache<Self, V>;
+
+    fn default_span(&self, _: TyCtxt<'_>) -> Span {
+        DUMMY_SP
+    }
+}

compiler/rustc_middle/src/ty/context.rs

@@ -32,7 +32,9 @@ use rustc_data_structures::sync::{
 use rustc_errors::{Applicability, Diag, DiagCtxtHandle, Diagnostic, MultiSpan};
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId, StableCrateIdMap};
-use rustc_hir::definitions::{DefPathData, Definitions, PerParentDisambiguatorState};
+use rustc_hir::definitions::{
+    DefPathData, Definitions, DisambiguatedDefPathData, PerParentDisambiguatorState,
+};
 use rustc_hir::intravisit::VisitorExt;
 use rustc_hir::lang_items::LangItem;
 use rustc_hir::limit::Limit;
@@ -52,7 +54,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, TaskDepsRef};
 use crate::ich::StableHashingContext;
 use crate::infer::canonical::{CanonicalParamEnvCache, CanonicalVarKind};
 use crate::lint::emit_lint_base;
@@ -1320,6 +1322,23 @@ impl<'tcx> TyCtxt<'tcx> {
     }
 }
 
+#[instrument(level = "trace", skip(tcx), ret)]
+fn create_def_raw_provider<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    (parent, data): (LocalDefId, DisambiguatedDefPathData),
+) -> LocalDefId {
+    // 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.
+    tcx.untracked.definitions.write().create_def(parent, data)
+}
+
 impl<'tcx> TyCtxtAt<'tcx> {
     /// Create a new definition within the incr. comp. engine.
     pub fn create_def(
@@ -1349,25 +1368,29 @@ 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 {
+            // `create_def_raw` is a query, so it can be replayed by the dep-graph engine.
+            // However, we may invoke it multiple times with the same `(parent, data)` pair,
+            // and we expect to create *different* definitions from them.
+            //
+            // In order to make this compatible with the general model of queries, we add
+            // additional information which must change at each call.
+            TaskDepsRef::Allow(..) => self.create_def_raw((parent, data)),
+
+            // 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)
+            }
+        });
 
         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
@@ -2801,4 +2824,5 @@ pub fn provide(providers: &mut Providers) {
         tcx.lang_items().panic_impl().is_some_and(|did| did.is_local())
     };
     providers.source_span = |tcx, def_id| tcx.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP);
+    providers.create_def_raw = create_def_raw_provider;
 }