Make create_def a side effect instead of marking the entire query as always red

compiler/rustc_hir/src/definitions.rs

@@ -344,7 +344,11 @@ impl Definitions {
     }
 
     /// Adds a definition with a parent definition.
-    pub fn create_def(&mut self, parent: LocalDefId, data: DefPathData) -> LocalDefId {
+    pub fn create_def(
+        &mut self,
+        parent: LocalDefId,
+        data: DefPathData,
+    ) -> (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!(
@@ -373,7 +377,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_hir_analysis/src/lib.rs

@@ -172,6 +172,15 @@ pub fn check_crate(tcx: TyCtxt<'_>) {
         }
     });
 
+    // Make sure we actually have a value for static items, as they aren't cached in incremental.
+    // While we could just wait for codegen to invoke this, the definitions freeze below will cause
+    // that to ICE, because evaluating statics can create more items.
+    tcx.hir().par_body_owners(|item_def_id| {
+        if let DefKind::Static { .. } = tcx.def_kind(item_def_id) {
+            let _ = tcx.eval_static_initializer(item_def_id);
+        }
+    });
+
     // FIXME: Remove this when we implement creating `DefId`s
     // for anon constants during their parents' typeck.
     // Typeck all body owners in parallel will produce queries

compiler/rustc_interface/src/callbacks.rs

@@ -41,8 +41,9 @@ fn track_span_parent(def_id: rustc_span::def_id::LocalDefId) {
 fn track_diagnostic<R>(diagnostic: DiagInner, f: &mut dyn FnMut(DiagInner) -> R) -> R {
     tls::with_context_opt(|icx| {
         if let Some(icx) = icx {
-            if let Some(diagnostics) = icx.diagnostics {
-                diagnostics.lock().extend(Some(diagnostic.clone()));
+            if let Some(side_effects) = icx.side_effects {
+                let diagnostic = diagnostic.clone();
+                side_effects.lock().diagnostics.push(diagnostic);
             }
 
             // Diagnostics are tracked, we can ignore the dependency.

compiler/rustc_middle/src/query/mod.rs

@@ -1116,7 +1116,6 @@ rustc_queries! {
             "evaluating initializer of static `{}`",
             tcx.def_path_str(key)
         }
-        cache_on_disk_if { key.is_local() }
         separate_provide_extern
         feedable
     }
@@ -1844,6 +1843,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

@@ -39,7 +39,7 @@ use rustc_hir::{self as hir, Attribute, HirId, Node, TraitCandidate};
 use rustc_index::IndexVec;
 use rustc_macros::{HashStable, TyDecodable, TyEncodable};
 use rustc_query_system::cache::WithDepNode;
-use rustc_query_system::dep_graph::DepNodeIndex;
+use rustc_query_system::dep_graph::{DepNodeIndex, TaskDepsRef};
 use rustc_query_system::ich::StableHashingContext;
 use rustc_serialize::opaque::{FileEncodeResult, FileEncoder};
 use rustc_session::config::CrateType;
@@ -50,10 +50,8 @@ use rustc_span::def_id::{CRATE_DEF_ID, DefPathHash, StableCrateId};
 use rustc_span::symbol::{Ident, Symbol, kw, sym};
 use rustc_span::{DUMMY_SP, Span};
 use rustc_type_ir::TyKind::*;
-use rustc_type_ir::fold::TypeFoldable;
-use rustc_type_ir::lang_items::TraitSolverLangItem;
-pub use rustc_type_ir::lift::Lift;
-use rustc_type_ir::{CollectAndApply, Interner, TypeFlags, WithCachedTypeInfo, search_graph};
+use rustc_type_ir::WithCachedTypeInfo;
+use rustc_type_ir::{CollectAndApply, Interner, TypeFlags};
 use tracing::{debug, instrument};
 
 use crate::arena::Arena;
@@ -1795,34 +1793,71 @@ 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,
         name: Symbol,
         def_kind: DefKind,
     ) -> TyCtxtFeed<'tcx, LocalDefId> {
         let data = def_kind.def_path_data(name);
-        // The following call has the side effect of modifying the tables inside `definitions`.
+        // The following create_def calls have 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 before the construction of the TyCtxt,
+        // - has been created when marking a query as green (recreating definitions it created in the actual run),
         // - 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.
         //
         // This call also writes to the value of `source_span` and `expn_that_defined` queries.
         // This is fine because:
         // - those queries are `eval_always` so we won't miss their result changing;
         // - this write will have happened before these queries are called.
-        let def_id = self.untracked.definitions.write().create_def(parent, data);
-
-        // 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);
+        let def_id = tls::with_context(|icx| {
+            match icx.task_deps {
+                // Always gets rerun anyway, so nothing to replay
+                TaskDepsRef::EvalAlways => {
+                    let def_id = self.untracked.definitions.write().create_def(parent, data).0;
+                    trace!(?def_id, "eval always");
+                    def_id
+                }
+                // Top-level queries like the resolver get rerun every time anyway
+                TaskDepsRef::Ignore => {
+                    let def_id = self.untracked.definitions.write().create_def(parent, data).0;
+                    trace!(?def_id, "ignore");
+                    def_id
+                }
+                TaskDepsRef::Forbid => bug!(
+                    "cannot create definition {parent:?}, {name:?}, {def_kind:?} without being able to register task dependencies"
+                ),
+                TaskDepsRef::Allow(_) => {
+                    let (def_id, hash) =
+                        self.untracked.definitions.write().create_def(parent, data);
+                    trace!(?def_id, "record side effects");
+
+                    icx.side_effects.as_ref().unwrap().lock().definitions.push(DefIdInfo {
+                        parent,
+                        data,
+                        hash,
+                    });
+                    def_id
+                }
+                TaskDepsRef::Replay { prev_side_effects, created_def_ids } => {
+                    trace!(?created_def_ids, "replay side effects");
+                    trace!("num_defs : {}", prev_side_effects.definitions.len());
+                    let index = created_def_ids.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+                    let prev_info = &prev_side_effects.definitions[index];
+                    let def_id = self.untracked.definitions.read().local_def_path_hash_to_def_id(
+                        prev_info.hash,
+                        &"should have already recreated def id in try_mark_green",
+                    );
+                    assert_eq!(prev_info.data, data);
+                    assert_eq!(prev_info.parent, parent);
+                    def_id
+                }
+            }
+        });
 
         let feed = TyCtxtFeed { tcx: self, key: def_id };
         feed.def_kind(def_kind);

compiler/rustc_middle/src/ty/context/tls.rs

@@ -1,8 +1,7 @@
 use std::{mem, ptr};
 
 use rustc_data_structures::sync::{self, Lock};
-use rustc_errors::DiagInner;
-use thin_vec::ThinVec;
+use rustc_query_system::query::QuerySideEffects;
 
 use super::{GlobalCtxt, TyCtxt};
 use crate::dep_graph::TaskDepsRef;
@@ -24,7 +23,7 @@ pub struct ImplicitCtxt<'a, 'tcx> {
 
     /// Where to store diagnostics for the current query job, if any.
     /// This is updated by `JobOwner::start` in `ty::query::plumbing` when executing a query.
-    pub diagnostics: Option<&'a Lock<ThinVec<DiagInner>>>,
+    pub side_effects: Option<&'a Lock<QuerySideEffects>>,
 
     /// Used to prevent queries from calling too deeply.
     pub query_depth: usize,
@@ -40,7 +39,7 @@ impl<'a, 'tcx> ImplicitCtxt<'a, 'tcx> {
         ImplicitCtxt {
             tcx,
             query: None,
-            diagnostics: None,
+            side_effects: None,
             query_depth: 0,
             task_deps: TaskDepsRef::Ignore,
         }

compiler/rustc_query_impl/src/plumbing.rs

@@ -7,7 +7,6 @@ use std::num::NonZero;
 use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher};
 use rustc_data_structures::sync::Lock;
 use rustc_data_structures::unord::UnordMap;
-use rustc_errors::DiagInner;
 use rustc_index::Idx;
 use rustc_middle::bug;
 use rustc_middle::dep_graph::{
@@ -24,14 +23,13 @@ use rustc_middle::ty::{self, TyCtxt, TyEncoder};
 use rustc_query_system::dep_graph::{DepNodeParams, HasDepContext};
 use rustc_query_system::ich::StableHashingContext;
 use rustc_query_system::query::{
-    QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap, QuerySideEffects, QueryStackFrame,
-    force_query,
+    DefIdInfo, QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap, QuerySideEffects,
+    QueryStackFrame, force_query,
 };
 use rustc_query_system::{LayoutOfDepth, QueryOverflow};
 use rustc_serialize::{Decodable, Encodable};
 use rustc_session::Limit;
 use rustc_span::def_id::LOCAL_CRATE;
-use thin_vec::ThinVec;
 
 use crate::QueryConfigRestored;
 
@@ -127,7 +125,7 @@ impl QueryContext for QueryCtxt<'_> {
         self,
         token: QueryJobId,
         depth_limit: bool,
-        diagnostics: Option<&Lock<ThinVec<DiagInner>>>,
+        side_effects: Option<&Lock<QuerySideEffects>>,
         compute: impl FnOnce() -> R,
     ) -> R {
         // The `TyCtxt` stored in TLS has the same global interner lifetime
@@ -142,7 +140,7 @@ impl QueryContext for QueryCtxt<'_> {
             let new_icx = ImplicitCtxt {
                 tcx: self.tcx,
                 query: Some(token),
-                diagnostics,
+                side_effects,
                 query_depth: current_icx.query_depth + depth_limit as usize,
                 task_deps: current_icx.task_deps,
             };
@@ -174,6 +172,21 @@ impl QueryContext for QueryCtxt<'_> {
             crate_name: self.crate_name(LOCAL_CRATE),
         });
     }
+
+    #[tracing::instrument(level = "trace", skip(self, side_effects))]
+    fn apply_side_effects(self, side_effects: QuerySideEffects) {
+        let dcx = self.dep_context().sess().dcx();
+        let QuerySideEffects { diagnostics, definitions } = side_effects;
+
+        for diagnostic in diagnostics {
+            dcx.emit_diagnostic(diagnostic);
+        }
+
+        for DefIdInfo { parent, data, hash } in definitions {
+            let (_def_id, h) = self.tcx.untracked().definitions.write().create_def(parent, data);
+            debug_assert_eq!(h, hash);
+        }
+    }
 }
 
 pub(super) fn try_mark_green<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &dep_graph::DepNode) -> bool {

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -11,7 +11,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::profiling::{QueryInvocationId, SelfProfilerRef};
 use rustc_data_structures::sharded::{self, Sharded};
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
-use rustc_data_structures::sync::{AtomicU32, AtomicU64, Lock, Lrc};
+use rustc_data_structures::sync::{AtomicU32, AtomicU64, AtomicUsize, Lock, Lrc};
 use rustc_data_structures::unord::UnordMap;
 use rustc_index::IndexVec;
 use rustc_macros::{Decodable, Encodable};
@@ -224,6 +224,15 @@ impl<D: Deps> DepGraph<D> {
         D::with_deps(TaskDepsRef::Ignore, op)
     }
 
+    pub(crate) fn with_replay<R>(
+        &self,
+        prev_side_effects: &QuerySideEffects,
+        created_def_ids: &AtomicUsize,
+        op: impl FnOnce() -> R,
+    ) -> R {
+        D::with_deps(TaskDepsRef::Replay { prev_side_effects, created_def_ids }, 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.
@@ -278,6 +287,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,
@@ -477,6 +487,12 @@ impl<D: Deps> DepGraph<D> {
                         return;
                     }
                     TaskDepsRef::Ignore => return,
+                    // We don't need to record dependencies when rerunning a query
+                    // because we have no disk cache entry to load. The dependencies
+                    // are preserved.
+                    // FIXME: assert that the dependencies don't change instead of
+                    // recording them.
+                    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)
@@ -583,6 +599,7 @@ impl<D: Deps> DepGraph<D> {
                     edges.push(DepNodeIndex::FOREVER_RED_NODE);
                 }
                 TaskDepsRef::Ignore => {}
+                TaskDepsRef::Replay { .. } => {}
                 TaskDepsRef::Forbid => {
                     panic!("Cannot summarize when dependencies are not recorded.")
                 }
@@ -892,7 +909,7 @@ impl<D: Deps> DepGraphData<D> {
         Some(dep_node_index)
     }
 
-    /// Atomically emits some loaded diagnostics.
+    /// Atomically emits some loaded side effects.
     /// This may be called concurrently on multiple threads for the same dep node.
     #[cold]
     #[inline(never)]
@@ -908,14 +925,10 @@ impl<D: Deps> DepGraphData<D> {
             // We were the first to insert the node in the set so this thread
             // must process side effects
 
-            // Promote the previous diagnostics to the current session.
+            // Promote the previous side effects to the current session.
             qcx.store_side_effects(dep_node_index, side_effects.clone());
 
-            let dcx = qcx.dep_context().sess().dcx();
-
-            for diagnostic in side_effects.diagnostics {
-                dcx.emit_diagnostic(diagnostic);
-            }
+            qcx.apply_side_effects(side_effects);
         }
     }
 }
@@ -1288,6 +1301,18 @@ pub enum TaskDepsRef<'a> {
     /// to ensure that the decoding process doesn't itself
     /// require the execution of any queries.
     Forbid,
+    /// Side effects from the previous run made available to
+    /// queries when they are reexecuted because their result was not
+    /// available in the cache. Whenever the query creates a new `DefId`,
+    /// it is checked against the entries in `QuerySideEffects::definitions`
+    /// to ensure that the new `DefId`s are the same as the ones that were
+    /// created the last time the query was executed.
+    Replay {
+        prev_side_effects: &'a QuerySideEffects,
+        /// Every new `DefId` is pushed here so we can check
+        /// that they match the cached ones.
+        created_def_ids: &'a AtomicUsize,
+    },
 }
 
 #[derive(Debug)]