From 3ca5220114c1e690d876aa46c3aa0f6ec594b9c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= <john.kare.alsaker@gmail.com>
Date: Thu, 7 Mar 2024 06:47:08 +0100
Subject: [PATCH 1/4] Represent diagnostic side effects as dep nodes

---
 compiler/rustc_interface/src/callbacks.rs     |   5 +-
 .../rustc_middle/src/dep_graph/dep_node.rs    |   1 +
 compiler/rustc_middle/src/dep_graph/mod.rs    |   1 +
 .../rustc_middle/src/query/on_disk_cache.rs   |  20 +---
 compiler/rustc_middle/src/ty/context/tls.rs   |  16 +--
 compiler/rustc_query_impl/src/plumbing.rs     |  48 ++++----
 .../src/dep_graph/dep_node.rs                 |   5 +-
 .../rustc_query_system/src/dep_graph/graph.rs | 105 +++++++++++-------
 .../rustc_query_system/src/dep_graph/mod.rs   |  12 +-
 compiler/rustc_query_system/src/query/mod.rs  |  45 ++------
 .../rustc_query_system/src/query/plumbing.rs  |  60 ++++------
 11 files changed, 138 insertions(+), 180 deletions(-)

diff --git a/compiler/rustc_interface/src/callbacks.rs b/compiler/rustc_interface/src/callbacks.rs
index f66b9eb3a2856..7c6b7157f71a5 100644
--- a/compiler/rustc_interface/src/callbacks.rs
+++ b/compiler/rustc_interface/src/callbacks.rs
@@ -14,6 +14,7 @@ use std::fmt;
 use rustc_errors::{DiagInner, TRACK_DIAGNOSTIC};
 use rustc_middle::dep_graph::{DepNodeExt, TaskDepsRef};
 use rustc_middle::ty::tls;
+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};
 
@@ -41,9 +42,7 @@ 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()));
-            }
+            icx.tcx.dep_graph.record_diagnostic(QueryCtxt::new(icx.tcx), &diagnostic);
 
             // Diagnostics are tracked, we can ignore the dependency.
             let icx = tls::ImplicitCtxt { task_deps: TaskDepsRef::Ignore, ..icx.clone() };
diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs
index 7525aeb922729..f967d8b92c71c 100644
--- a/compiler/rustc_middle/src/dep_graph/dep_node.rs
+++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs
@@ -79,6 +79,7 @@ rustc_query_append!(define_dep_nodes![
     [] fn Null() -> (),
     /// We use this to create a forever-red node.
     [] fn Red() -> (),
+    [] fn SideEffect() -> (),
     [] fn TraitSelect() -> (),
     [] fn CompileCodegenUnit() -> (),
     [] fn CompileMonoItem() -> (),
diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs
index 2090c3e6da6fa..c927538b4cf37 100644
--- a/compiler/rustc_middle/src/dep_graph/mod.rs
+++ b/compiler/rustc_middle/src/dep_graph/mod.rs
@@ -46,6 +46,7 @@ impl Deps for DepsType {
 
     const DEP_KIND_NULL: DepKind = dep_kinds::Null;
     const DEP_KIND_RED: DepKind = dep_kinds::Red;
+    const DEP_KIND_SIDE_EFFECT: DepKind = dep_kinds::SideEffect;
     const DEP_KIND_MAX: u16 = dep_node::DEP_KIND_VARIANTS - 1;
 }
 
diff --git a/compiler/rustc_middle/src/query/on_disk_cache.rs b/compiler/rustc_middle/src/query/on_disk_cache.rs
index d9035efaf56d0..b4c5d1c6c0558 100644
--- a/compiler/rustc_middle/src/query/on_disk_cache.rs
+++ b/compiler/rustc_middle/src/query/on_disk_cache.rs
@@ -357,11 +357,10 @@ impl OnDiskCache {
         &self,
         tcx: TyCtxt<'_>,
         dep_node_index: SerializedDepNodeIndex,
-    ) -> QuerySideEffects {
+    ) -> Option<QuerySideEffects> {
         let side_effects: Option<QuerySideEffects> =
             self.load_indexed(tcx, dep_node_index, &self.prev_side_effects_index);
-
-        side_effects.unwrap_or_default()
+        side_effects
     }
 
     /// Stores a `QuerySideEffects` emitted during the current compilation session.
@@ -395,21 +394,6 @@ impl OnDiskCache {
         opt_value
     }
 
-    /// Stores side effect emitted during computation of an anonymous query.
-    /// Since many anonymous queries can share the same `DepNode`, we aggregate
-    /// them -- as opposed to regular queries where we assume that there is a
-    /// 1:1 relationship between query-key and `DepNode`.
-    pub fn store_side_effects_for_anon_node(
-        &self,
-        dep_node_index: DepNodeIndex,
-        side_effects: QuerySideEffects,
-    ) {
-        let mut current_side_effects = self.current_side_effects.borrow_mut();
-
-        let x = current_side_effects.entry(dep_node_index).or_default();
-        x.append(side_effects);
-    }
-
     fn load_indexed<'tcx, T>(
         &self,
         tcx: TyCtxt<'tcx>,
diff --git a/compiler/rustc_middle/src/ty/context/tls.rs b/compiler/rustc_middle/src/ty/context/tls.rs
index eaab8474dd20c..5fc80bc793673 100644
--- a/compiler/rustc_middle/src/ty/context/tls.rs
+++ b/compiler/rustc_middle/src/ty/context/tls.rs
@@ -1,8 +1,6 @@
 use std::{mem, ptr};
 
-use rustc_data_structures::sync::{self, Lock};
-use rustc_errors::DiagInner;
-use thin_vec::ThinVec;
+use rustc_data_structures::sync;
 
 use super::{GlobalCtxt, TyCtxt};
 use crate::dep_graph::TaskDepsRef;
@@ -22,10 +20,6 @@ pub struct ImplicitCtxt<'a, 'tcx> {
     /// `ty::query::plumbing` when executing a query.
     pub query: Option<QueryJobId>,
 
-    /// 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>>>,
-
     /// Used to prevent queries from calling too deeply.
     pub query_depth: usize,
 
@@ -37,13 +31,7 @@ pub struct ImplicitCtxt<'a, 'tcx> {
 impl<'a, 'tcx> ImplicitCtxt<'a, 'tcx> {
     pub fn new(gcx: &'tcx GlobalCtxt<'tcx>) -> Self {
         let tcx = TyCtxt { gcx };
-        ImplicitCtxt {
-            tcx,
-            query: None,
-            diagnostics: None,
-            query_depth: 0,
-            task_deps: TaskDepsRef::Ignore,
-        }
+        ImplicitCtxt { tcx, query: None, query_depth: 0, task_deps: TaskDepsRef::Ignore }
     }
 }
 
diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs
index f98d6421307ad..47158f6d7aaf7 100644
--- a/compiler/rustc_query_impl/src/plumbing.rs
+++ b/compiler/rustc_query_impl/src/plumbing.rs
@@ -5,9 +5,7 @@
 use std::num::NonZero;
 
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
-use rustc_data_structures::sync::Lock;
 use rustc_data_structures::unord::UnordMap;
-use rustc_errors::DiagInner;
 use rustc_hashes::Hash64;
 use rustc_index::Idx;
 use rustc_middle::bug;
@@ -32,7 +30,6 @@ use rustc_query_system::{QueryOverflow, QueryOverflowNote};
 use rustc_serialize::{Decodable, Encodable};
 use rustc_session::Limit;
 use rustc_span::def_id::LOCAL_CRATE;
-use thin_vec::ThinVec;
 
 use crate::QueryConfigRestored;
 
@@ -92,12 +89,14 @@ impl QueryContext for QueryCtxt<'_> {
     }
 
     // Interactions with on_disk_cache
-    fn load_side_effects(self, prev_dep_node_index: SerializedDepNodeIndex) -> QuerySideEffects {
+    fn load_side_effects(
+        self,
+        prev_dep_node_index: SerializedDepNodeIndex,
+    ) -> Option<QuerySideEffects> {
         self.query_system
             .on_disk_cache
             .as_ref()
-            .map(|c| c.load_side_effects(self.tcx, prev_dep_node_index))
-            .unwrap_or_default()
+            .and_then(|c| c.load_side_effects(self.tcx, prev_dep_node_index))
     }
 
     #[inline(never)]
@@ -108,27 +107,13 @@ impl QueryContext for QueryCtxt<'_> {
         }
     }
 
-    #[inline(never)]
-    #[cold]
-    fn store_side_effects_for_anon_node(
-        self,
-        dep_node_index: DepNodeIndex,
-        side_effects: QuerySideEffects,
-    ) {
-        if let Some(c) = self.query_system.on_disk_cache.as_ref() {
-            c.store_side_effects_for_anon_node(dep_node_index, side_effects)
-        }
-    }
-
     /// Executes a job by changing the `ImplicitCtxt` to point to the
-    /// new query job while it executes. It returns the diagnostics
-    /// captured during execution and the actual result.
+    /// new query job while it executes.
     #[inline(always)]
     fn start_query<R>(
         self,
         token: QueryJobId,
         depth_limit: bool,
-        diagnostics: Option<&Lock<ThinVec<DiagInner>>>,
         compute: impl FnOnce() -> R,
     ) -> R {
         // The `TyCtxt` stored in TLS has the same global interner lifetime
@@ -143,7 +128,6 @@ impl QueryContext for QueryCtxt<'_> {
             let new_icx = ImplicitCtxt {
                 tcx: self.tcx,
                 query: Some(token),
-                diagnostics,
                 query_depth: current_icx.query_depth + depth_limit as usize,
                 task_deps: current_icx.task_deps,
             };
@@ -500,7 +484,7 @@ where
         is_anon,
         is_eval_always,
         fingerprint_style,
-        force_from_dep_node: Some(|tcx, dep_node| {
+        force_from_dep_node: Some(|tcx, dep_node, _| {
             force_from_dep_node(Q::config(tcx), tcx, dep_node)
         }),
         try_load_from_on_disk_cache: Some(|tcx, dep_node| {
@@ -802,7 +786,7 @@ macro_rules! define_queries {
                     is_anon: false,
                     is_eval_always: false,
                     fingerprint_style: FingerprintStyle::Unit,
-                    force_from_dep_node: Some(|_, dep_node| bug!("force_from_dep_node: encountered {:?}", dep_node)),
+                    force_from_dep_node: Some(|_, dep_node, _| bug!("force_from_dep_node: encountered {:?}", dep_node)),
                     try_load_from_on_disk_cache: None,
                     name: &"Null",
                 }
@@ -814,12 +798,26 @@ macro_rules! define_queries {
                     is_anon: false,
                     is_eval_always: false,
                     fingerprint_style: FingerprintStyle::Unit,
-                    force_from_dep_node: Some(|_, dep_node| bug!("force_from_dep_node: encountered {:?}", dep_node)),
+                    force_from_dep_node: Some(|_, dep_node, _| bug!("force_from_dep_node: encountered {:?}", dep_node)),
                     try_load_from_on_disk_cache: None,
                     name: &"Red",
                 }
             }
 
+            pub(crate) fn SideEffect<'tcx>() -> DepKindStruct<'tcx> {
+                DepKindStruct {
+                    is_anon: false,
+                    is_eval_always: false,
+                    fingerprint_style: FingerprintStyle::Unit,
+                    force_from_dep_node: Some(|tcx, _, prev_index| {
+                        tcx.dep_graph.force_diagnostic_node(QueryCtxt::new(tcx), prev_index);
+                        true
+                    }),
+                    try_load_from_on_disk_cache: None,
+                    name: &"SideEffect",
+                }
+            }
+
             pub(crate) fn TraitSelect<'tcx>() -> DepKindStruct<'tcx> {
                 DepKindStruct {
                     is_anon: true,
diff --git a/compiler/rustc_query_system/src/dep_graph/dep_node.rs b/compiler/rustc_query_system/src/dep_graph/dep_node.rs
index 9d3368607a21c..c0b3bd43e25a6 100644
--- a/compiler/rustc_query_system/src/dep_graph/dep_node.rs
+++ b/compiler/rustc_query_system/src/dep_graph/dep_node.rs
@@ -64,7 +64,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableOrd,
 use rustc_hir::definitions::DefPathHash;
 use rustc_macros::{Decodable, Encodable};
 
-use super::{DepContext, FingerprintStyle};
+use super::{DepContext, FingerprintStyle, SerializedDepNodeIndex};
 use crate::ich::StableHashingContext;
 
 /// This serves as an index into arrays built by `make_dep_kind_array`.
@@ -275,7 +275,8 @@ pub struct DepKindStruct<Tcx: DepContext> {
     /// with kind `MirValidated`, we know that the GUID/fingerprint of the `DepNode`
     /// is actually a `DefPathHash`, and can therefore just look up the corresponding
     /// `DefId` in `tcx.def_path_hash_to_def_id`.
-    pub force_from_dep_node: Option<fn(tcx: Tcx, dep_node: DepNode) -> bool>,
+    pub force_from_dep_node:
+        Option<fn(tcx: Tcx, dep_node: DepNode, prev_index: SerializedDepNodeIndex) -> bool>,
 
     /// Invoke a query to put the on-disk cached value in memory.
     pub try_load_from_on_disk_cache: Option<fn(Tcx, DepNode)>,
diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs
index 3109d53cd2caa..de681328bcb51 100644
--- a/compiler/rustc_query_system/src/dep_graph/graph.rs
+++ b/compiler/rustc_query_system/src/dep_graph/graph.rs
@@ -5,13 +5,14 @@ use std::marker::PhantomData;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicU32, Ordering};
 
-use rustc_data_structures::fingerprint::Fingerprint;
+use rustc_data_structures::fingerprint::{Fingerprint, PackedFingerprint};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::profiling::{QueryInvocationId, SelfProfilerRef};
 use rustc_data_structures::sharded::{self, ShardedHashMap};
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::sync::{AtomicU64, Lock};
 use rustc_data_structures::unord::UnordMap;
+use rustc_errors::DiagInner;
 use rustc_index::IndexVec;
 use rustc_macros::{Decodable, Encodable};
 use rustc_serialize::opaque::{FileEncodeResult, FileEncoder};
@@ -91,8 +92,6 @@ pub(crate) struct DepGraphData<D: Deps> {
 
     colors: DepNodeColorMap,
 
-    processed_side_effects: Lock<FxHashSet<DepNodeIndex>>,
-
     /// When we load, there may be `.o` files, cached MIR, or other such
     /// things available to us. If we find that they are not dirty, we
     /// load the path to the file storing those work-products here into
@@ -174,7 +173,6 @@ impl<D: Deps> DepGraph<D> {
                 previous_work_products: prev_work_products,
                 dep_node_debug: Default::default(),
                 current,
-                processed_side_effects: Default::default(),
                 previous: prev_graph,
                 colors,
                 debug_loaded_from_disk: Default::default(),
@@ -535,6 +533,24 @@ impl<D: Deps> DepGraph<D> {
         }
     }
 
+    #[inline]
+    pub fn record_diagnostic<Qcx: QueryContext>(&self, qcx: Qcx, diagnostic: &DiagInner) {
+        if let Some(ref data) = self.data {
+            self.read_index(data.encode_diagnostic(qcx, diagnostic));
+        }
+    }
+
+    #[inline]
+    pub fn force_diagnostic_node<Qcx: QueryContext>(
+        &self,
+        qcx: Qcx,
+        prev_index: SerializedDepNodeIndex,
+    ) {
+        if let Some(ref data) = self.data {
+            data.force_diagnostic_node(qcx, prev_index);
+        }
+    }
+
     /// Create a node when we force-feed a value into the query cache.
     /// This is used to remove cycles during type-checking const generic parameters.
     ///
@@ -656,6 +672,48 @@ impl<D: Deps> DepGraphData<D> {
     pub(crate) fn mark_debug_loaded_from_disk(&self, dep_node: DepNode) {
         self.debug_loaded_from_disk.lock().insert(dep_node);
     }
+
+    #[inline]
+    fn encode_diagnostic<Qcx: QueryContext>(
+        &self,
+        qcx: Qcx,
+        diagnostic: &DiagInner,
+    ) -> DepNodeIndex {
+        let dep_node_index = self.current.encoder.send(
+            DepNode {
+                kind: D::DEP_KIND_SIDE_EFFECT,
+                hash: PackedFingerprint::from(Fingerprint::ZERO),
+            },
+            Fingerprint::ZERO,
+            // We want the side effect node to always be red so it will be forced and emit the
+            // diagnostic.
+            std::iter::once(DepNodeIndex::FOREVER_RED_NODE).collect(),
+        );
+        let side_effects = QuerySideEffects { diagnostic: diagnostic.clone() };
+        qcx.store_side_effects(dep_node_index, side_effects);
+        dep_node_index
+    }
+
+    #[inline]
+    fn force_diagnostic_node<Qcx: QueryContext>(
+        &self,
+        qcx: Qcx,
+        prev_index: SerializedDepNodeIndex,
+    ) {
+        D::with_deps(TaskDepsRef::Ignore, || {
+            let side_effects = qcx.load_side_effects(prev_index).unwrap();
+
+            qcx.dep_context().sess().dcx().emit_diagnostic(side_effects.diagnostic.clone());
+
+            // Promote the previous diagnostics to the current session.
+            let index = self.current.promote_node_and_deps_to_current(&self.previous, prev_index);
+            // FIXME: Can this race with a parallel compiler?
+            qcx.store_side_effects(index, side_effects);
+
+            // Mark the node as green.
+            self.colors.insert(prev_index, DepNodeColor::Green(index));
+        })
+    }
 }
 
 impl<D: Deps> DepGraph<D> {
@@ -794,7 +852,7 @@ impl<D: Deps> DepGraphData<D> {
 
         // We failed to mark it green, so we try to force the query.
         debug!("trying to force dependency {dep_dep_node:?}");
-        if !qcx.dep_context().try_force_from_dep_node(*dep_dep_node, frame) {
+        if !qcx.dep_context().try_force_from_dep_node(*dep_dep_node, parent_dep_node_index, frame) {
             // The DepNode could not be forced.
             debug!("dependency {dep_dep_node:?} could not be forced");
             return None;
@@ -867,16 +925,6 @@ impl<D: Deps> DepGraphData<D> {
 
         // ... emitting any stored diagnostic ...
 
-        // FIXME: Store the fact that a node has diagnostics in a bit in the dep graph somewhere
-        // Maybe store a list on disk and encode this fact in the DepNodeState
-        let side_effects = qcx.load_side_effects(prev_dep_node_index);
-
-        if side_effects.maybe_any() {
-            qcx.dep_context().dep_graph().with_query_deserialization(|| {
-                self.emit_side_effects(qcx, dep_node_index, side_effects)
-            });
-        }
-
         // ... and finally storing a "Green" entry in the color map.
         // Multiple threads can all write the same color here
         self.colors.insert(prev_dep_node_index, DepNodeColor::Green(dep_node_index));
@@ -884,33 +932,6 @@ impl<D: Deps> DepGraphData<D> {
         debug!("successfully marked {dep_node:?} as green");
         Some(dep_node_index)
     }
-
-    /// Atomically emits some loaded diagnostics.
-    /// This may be called concurrently on multiple threads for the same dep node.
-    #[cold]
-    #[inline(never)]
-    fn emit_side_effects<Qcx: QueryContext<Deps = D>>(
-        &self,
-        qcx: Qcx,
-        dep_node_index: DepNodeIndex,
-        side_effects: QuerySideEffects,
-    ) {
-        let mut processed = self.processed_side_effects.lock();
-
-        if processed.insert(dep_node_index) {
-            // 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.
-            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);
-            }
-        }
-    }
 }
 
 impl<D: Deps> DepGraph<D> {
diff --git a/compiler/rustc_query_system/src/dep_graph/mod.rs b/compiler/rustc_query_system/src/dep_graph/mod.rs
index e564ae0cec7a8..e3d64d1c0f802 100644
--- a/compiler/rustc_query_system/src/dep_graph/mod.rs
+++ b/compiler/rustc_query_system/src/dep_graph/mod.rs
@@ -58,10 +58,15 @@ pub trait DepContext: Copy {
     /// dep-node or when the query kind outright does not support it.
     #[inline]
     #[instrument(skip(self, frame), level = "debug")]
-    fn try_force_from_dep_node(self, dep_node: DepNode, frame: Option<&MarkFrame<'_>>) -> bool {
+    fn try_force_from_dep_node(
+        self,
+        dep_node: DepNode,
+        prev_index: SerializedDepNodeIndex,
+        frame: Option<&MarkFrame<'_>>,
+    ) -> bool {
         let cb = self.dep_kind_info(dep_node.kind);
         if let Some(f) = cb.force_from_dep_node {
-            match panic::catch_unwind(panic::AssertUnwindSafe(|| f(self, dep_node))) {
+            match panic::catch_unwind(panic::AssertUnwindSafe(|| f(self, dep_node, prev_index))) {
                 Err(value) => {
                     if !value.is::<rustc_errors::FatalErrorMarker>() {
                         print_markframe_trace(self.dep_graph(), frame);
@@ -101,6 +106,9 @@ pub trait Deps {
     /// We use this to create a forever-red node.
     const DEP_KIND_RED: DepKind;
 
+    /// We use this to create a side effect node.
+    const DEP_KIND_SIDE_EFFECT: DepKind;
+
     /// This is the highest value a `DepKind` can have. It's used during encoding to
     /// pack information into the unused bits.
     const DEP_KIND_MAX: u16;
diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs
index 7490a3f35032e..ad246b3e8b10d 100644
--- a/compiler/rustc_query_system/src/query/mod.rs
+++ b/compiler/rustc_query_system/src/query/mod.rs
@@ -11,14 +11,12 @@ mod caches;
 pub use self::caches::{DefIdCache, DefaultCache, QueryCache, SingleCache, VecCache};
 
 mod config;
-use rustc_data_structures::sync::Lock;
 use rustc_errors::DiagInner;
 use rustc_hashes::Hash64;
 use rustc_hir::def::DefKind;
 use rustc_macros::{Decodable, Encodable};
 use rustc_span::Span;
 use rustc_span::def_id::DefId;
-use thin_vec::ThinVec;
 
 pub use self::config::{HashResult, QueryConfig};
 use crate::dep_graph::{DepKind, DepNodeIndex, HasDepContext, SerializedDepNodeIndex};
@@ -70,27 +68,12 @@ impl QueryStackFrame {
 /// This allows us to 'replay' changes to global state
 /// that would otherwise only occur if we actually
 /// executed the query method.
-#[derive(Debug, Clone, Default, Encodable, Decodable)]
+#[derive(Debug, Encodable, Decodable)]
 pub struct QuerySideEffects {
     /// Stores any diagnostics emitted during query execution.
     /// These diagnostics will be re-emitted if we mark
     /// the query as green.
-    pub(super) diagnostics: ThinVec<DiagInner>,
-}
-
-impl QuerySideEffects {
-    /// Returns true if there might be side effects.
-    #[inline]
-    pub fn maybe_any(&self) -> bool {
-        let QuerySideEffects { diagnostics } = self;
-        // Use `has_capacity` so that the destructor for `self.diagnostics` can be skipped
-        // if `maybe_any` is known to be false.
-        diagnostics.has_capacity()
-    }
-    pub fn append(&mut self, other: QuerySideEffects) {
-        let QuerySideEffects { diagnostics } = self;
-        diagnostics.extend(other.diagnostics);
-    }
+    pub(super) diagnostic: DiagInner,
 }
 
 pub trait QueryContext: HasDepContext {
@@ -102,28 +85,18 @@ pub trait QueryContext: HasDepContext {
     fn collect_active_jobs(self) -> QueryMap;
 
     /// Load side effects associated to the node in the previous session.
-    fn load_side_effects(self, prev_dep_node_index: SerializedDepNodeIndex) -> QuerySideEffects;
+    fn load_side_effects(
+        self,
+        prev_dep_node_index: SerializedDepNodeIndex,
+    ) -> Option<QuerySideEffects>;
 
     /// Register diagnostics for the given node, for use in next session.
     fn store_side_effects(self, dep_node_index: DepNodeIndex, side_effects: QuerySideEffects);
 
-    /// Register diagnostics for the given node, for use in next session.
-    fn store_side_effects_for_anon_node(
-        self,
-        dep_node_index: DepNodeIndex,
-        side_effects: QuerySideEffects,
-    );
-
     /// Executes a job by changing the `ImplicitCtxt` to point to the
-    /// new query job while it executes. It returns the diagnostics
-    /// captured during execution and the actual result.
-    fn start_query<R>(
-        self,
-        token: QueryJobId,
-        depth_limit: bool,
-        diagnostics: Option<&Lock<ThinVec<DiagInner>>>,
-        compute: impl FnOnce() -> R,
-    ) -> R;
+    /// new query job while it executes.
+    fn start_query<R>(self, token: QueryJobId, depth_limit: bool, compute: impl FnOnce() -> R)
+    -> R;
 
     fn depth_limit_error(self, job: QueryJobId);
 }
diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs
index 18aae8c00b247..7578cb5e2aebd 100644
--- a/compiler/rustc_query_system/src/query/plumbing.rs
+++ b/compiler/rustc_query_system/src/query/plumbing.rs
@@ -12,11 +12,9 @@ use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::sharded::Sharded;
 use rustc_data_structures::stack::ensure_sufficient_stack;
-use rustc_data_structures::sync::Lock;
 use rustc_data_structures::{outline, sync};
 use rustc_errors::{Diag, FatalError, StashKey};
 use rustc_span::{DUMMY_SP, Span};
-use thin_vec::ThinVec;
 use tracing::instrument;
 
 use super::QueryConfig;
@@ -25,9 +23,7 @@ use crate::dep_graph::{DepContext, DepGraphData, DepNode, DepNodeIndex, DepNodeP
 use crate::ich::StableHashingContext;
 use crate::query::caches::QueryCache;
 use crate::query::job::{QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryLatch, report_cycle};
-use crate::query::{
-    QueryContext, QueryMap, QuerySideEffects, QueryStackFrame, SerializedDepNodeIndex,
-};
+use crate::query::{QueryContext, QueryMap, QueryStackFrame, SerializedDepNodeIndex};
 
 pub struct QueryState<K> {
     active: Sharded<FxHashMap<K, QueryResult>>,
@@ -470,7 +466,7 @@ where
     }
 
     let prof_timer = qcx.dep_context().profiler().query_provider();
-    let result = qcx.start_query(job_id, query.depth_limit(), None, || query.compute(qcx, key));
+    let result = qcx.start_query(job_id, query.depth_limit(), || query.compute(qcx, key));
     let dep_node_index = qcx.dep_context().dep_graph().next_virtual_depnode_index();
     prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 
@@ -507,7 +503,7 @@ where
 
         // The diagnostics for this query will be promoted to the current session during
         // `try_mark_green()`, so we can ignore them here.
-        if let Some(ret) = qcx.start_query(job_id, false, None, || {
+        if let Some(ret) = qcx.start_query(job_id, false, || {
             try_load_from_disk_and_cache_in_memory(query, dep_graph_data, qcx, &key, dep_node)
         }) {
             return ret;
@@ -515,43 +511,31 @@ where
     }
 
     let prof_timer = qcx.dep_context().profiler().query_provider();
-    let diagnostics = Lock::new(ThinVec::new());
 
-    let (result, dep_node_index) =
-        qcx.start_query(job_id, query.depth_limit(), Some(&diagnostics), || {
-            if query.anon() {
-                return dep_graph_data.with_anon_task_inner(
-                    *qcx.dep_context(),
-                    query.dep_kind(),
-                    || query.compute(qcx, key),
-                );
-            }
+    let (result, dep_node_index) = qcx.start_query(job_id, query.depth_limit(), || {
+        if query.anon() {
+            return dep_graph_data.with_anon_task_inner(
+                *qcx.dep_context(),
+                query.dep_kind(),
+                || query.compute(qcx, key),
+            );
+        }
 
-            // `to_dep_node` is expensive for some `DepKind`s.
-            let dep_node =
-                dep_node_opt.unwrap_or_else(|| query.construct_dep_node(*qcx.dep_context(), &key));
+        // `to_dep_node` is expensive for some `DepKind`s.
+        let dep_node =
+            dep_node_opt.unwrap_or_else(|| query.construct_dep_node(*qcx.dep_context(), &key));
 
-            dep_graph_data.with_task(
-                dep_node,
-                (qcx, query),
-                key,
-                |(qcx, query), key| query.compute(qcx, key),
-                query.hash_result(),
-            )
-        });
+        dep_graph_data.with_task(
+            dep_node,
+            (qcx, query),
+            key,
+            |(qcx, query), key| query.compute(qcx, key),
+            query.hash_result(),
+        )
+    });
 
     prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 
-    let side_effects = QuerySideEffects { diagnostics: diagnostics.into_inner() };
-
-    if std::intrinsics::unlikely(side_effects.maybe_any()) {
-        if query.anon() {
-            qcx.store_side_effects_for_anon_node(dep_node_index, side_effects);
-        } else {
-            qcx.store_side_effects(dep_node_index, side_effects);
-        }
-    }
-
     (result, dep_node_index)
 }
 

From 453b51a65ab00939d2dce3de24ce38489b5c7eb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= <john.kare.alsaker@gmail.com>
Date: Fri, 14 Mar 2025 18:36:30 +0100
Subject: [PATCH 2/4] Rename `QuerySideEffects` to `QuerySideEffect`

---
 .../rustc_middle/src/query/on_disk_cache.rs   | 30 +++++++++----------
 compiler/rustc_query_impl/src/plumbing.rs     | 12 ++++----
 .../rustc_query_system/src/dep_graph/graph.rs | 16 ++++++----
 compiler/rustc_query_system/src/query/mod.rs  | 26 +++++++++-------
 compiler/rustc_session/src/options.rs         |  2 +-
 5 files changed, 47 insertions(+), 39 deletions(-)

diff --git a/compiler/rustc_middle/src/query/on_disk_cache.rs b/compiler/rustc_middle/src/query/on_disk_cache.rs
index b4c5d1c6c0558..ec5f8f619fe76 100644
--- a/compiler/rustc_middle/src/query/on_disk_cache.rs
+++ b/compiler/rustc_middle/src/query/on_disk_cache.rs
@@ -11,7 +11,7 @@ use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LOCAL_CRATE, LocalDefId, Stab
 use rustc_hir::definitions::DefPathHash;
 use rustc_index::{Idx, IndexVec};
 use rustc_macros::{Decodable, Encodable};
-use rustc_query_system::query::QuerySideEffects;
+use rustc_query_system::query::QuerySideEffect;
 use rustc_serialize::opaque::{FileEncodeResult, FileEncoder, IntEncodedWithFixedSize, MemDecoder};
 use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
 use rustc_session::Session;
@@ -55,9 +55,9 @@ pub struct OnDiskCache {
     // The complete cache data in serialized form.
     serialized_data: RwLock<Option<Mmap>>,
 
-    // Collects all `QuerySideEffects` created during the current compilation
+    // Collects all `QuerySideEffect` created during the current compilation
     // session.
-    current_side_effects: Lock<FxHashMap<DepNodeIndex, QuerySideEffects>>,
+    current_side_effects: Lock<FxHashMap<DepNodeIndex, QuerySideEffect>>,
 
     file_index_to_stable_id: FxHashMap<SourceFileIndex, EncodedSourceFileId>,
 
@@ -68,7 +68,7 @@ pub struct OnDiskCache {
     // `serialized_data`.
     query_result_index: FxHashMap<SerializedDepNodeIndex, AbsoluteBytePos>,
 
-    // A map from dep-node to the position of any associated `QuerySideEffects` in
+    // A map from dep-node to the position of any associated `QuerySideEffect` in
     // `serialized_data`.
     prev_side_effects_index: FxHashMap<SerializedDepNodeIndex, AbsoluteBytePos>,
 
@@ -270,10 +270,10 @@ impl OnDiskCache {
                 .current_side_effects
                 .borrow()
                 .iter()
-                .map(|(dep_node_index, side_effects)| {
+                .map(|(dep_node_index, side_effect)| {
                     let pos = AbsoluteBytePos::new(encoder.position());
                     let dep_node_index = SerializedDepNodeIndex::new(dep_node_index.index());
-                    encoder.encode_tagged(dep_node_index, side_effects);
+                    encoder.encode_tagged(dep_node_index, side_effect);
 
                     (dep_node_index, pos)
                 })
@@ -352,23 +352,23 @@ impl OnDiskCache {
         })
     }
 
-    /// Loads a `QuerySideEffects` created during the previous compilation session.
-    pub fn load_side_effects(
+    /// Loads a `QuerySideEffect` created during the previous compilation session.
+    pub fn load_side_effect(
         &self,
         tcx: TyCtxt<'_>,
         dep_node_index: SerializedDepNodeIndex,
-    ) -> Option<QuerySideEffects> {
-        let side_effects: Option<QuerySideEffects> =
+    ) -> Option<QuerySideEffect> {
+        let side_effect: Option<QuerySideEffect> =
             self.load_indexed(tcx, dep_node_index, &self.prev_side_effects_index);
-        side_effects
+        side_effect
     }
 
-    /// Stores a `QuerySideEffects` emitted during the current compilation session.
-    /// Anything stored like this will be available via `load_side_effects` in
+    /// Stores a `QuerySideEffect` emitted during the current compilation session.
+    /// Anything stored like this will be available via `load_side_effect` in
     /// the next compilation session.
-    pub fn store_side_effects(&self, dep_node_index: DepNodeIndex, side_effects: QuerySideEffects) {
+    pub fn store_side_effect(&self, dep_node_index: DepNodeIndex, side_effect: QuerySideEffect) {
         let mut current_side_effects = self.current_side_effects.borrow_mut();
-        let prev = current_side_effects.insert(dep_node_index, side_effects);
+        let prev = current_side_effects.insert(dep_node_index, side_effect);
         debug_assert!(prev.is_none());
     }
 
diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs
index 47158f6d7aaf7..e11bd6437d7b8 100644
--- a/compiler/rustc_query_impl/src/plumbing.rs
+++ b/compiler/rustc_query_impl/src/plumbing.rs
@@ -23,7 +23,7 @@ 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,
+    QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap, QuerySideEffect, QueryStackFrame,
     force_query,
 };
 use rustc_query_system::{QueryOverflow, QueryOverflowNote};
@@ -89,21 +89,21 @@ impl QueryContext for QueryCtxt<'_> {
     }
 
     // Interactions with on_disk_cache
-    fn load_side_effects(
+    fn load_side_effect(
         self,
         prev_dep_node_index: SerializedDepNodeIndex,
-    ) -> Option<QuerySideEffects> {
+    ) -> Option<QuerySideEffect> {
         self.query_system
             .on_disk_cache
             .as_ref()
-            .and_then(|c| c.load_side_effects(self.tcx, prev_dep_node_index))
+            .and_then(|c| c.load_side_effect(self.tcx, prev_dep_node_index))
     }
 
     #[inline(never)]
     #[cold]
-    fn store_side_effects(self, dep_node_index: DepNodeIndex, side_effects: QuerySideEffects) {
+    fn store_side_effect(self, dep_node_index: DepNodeIndex, side_effect: QuerySideEffect) {
         if let Some(c) = self.query_system.on_disk_cache.as_ref() {
-            c.store_side_effects(dep_node_index, side_effects)
+            c.store_side_effect(dep_node_index, side_effect)
         }
     }
 
diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs
index de681328bcb51..dbc7f034a6eb0 100644
--- a/compiler/rustc_query_system/src/dep_graph/graph.rs
+++ b/compiler/rustc_query_system/src/dep_graph/graph.rs
@@ -25,7 +25,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, QuerySideEffects};
+use crate::query::{QueryContext, QuerySideEffect};
 
 #[derive(Clone)]
 pub struct DepGraph<D: Deps> {
@@ -689,8 +689,8 @@ impl<D: Deps> DepGraphData<D> {
             // diagnostic.
             std::iter::once(DepNodeIndex::FOREVER_RED_NODE).collect(),
         );
-        let side_effects = QuerySideEffects { diagnostic: diagnostic.clone() };
-        qcx.store_side_effects(dep_node_index, side_effects);
+        let side_effect = QuerySideEffect::Diagnostic(diagnostic.clone());
+        qcx.store_side_effect(dep_node_index, side_effect);
         dep_node_index
     }
 
@@ -701,14 +701,18 @@ impl<D: Deps> DepGraphData<D> {
         prev_index: SerializedDepNodeIndex,
     ) {
         D::with_deps(TaskDepsRef::Ignore, || {
-            let side_effects = qcx.load_side_effects(prev_index).unwrap();
+            let side_effect = qcx.load_side_effect(prev_index).unwrap();
 
-            qcx.dep_context().sess().dcx().emit_diagnostic(side_effects.diagnostic.clone());
+            match &side_effect {
+                QuerySideEffect::Diagnostic(diagnostic) => {
+                    qcx.dep_context().sess().dcx().emit_diagnostic(diagnostic.clone());
+                }
+            }
 
             // Promote the previous diagnostics to the current session.
             let index = self.current.promote_node_and_deps_to_current(&self.previous, prev_index);
             // FIXME: Can this race with a parallel compiler?
-            qcx.store_side_effects(index, side_effects);
+            qcx.store_side_effect(index, side_effect);
 
             // Mark the node as green.
             self.colors.insert(prev_index, DepNodeColor::Green(index));
diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs
index ad246b3e8b10d..2ed0c810b7516 100644
--- a/compiler/rustc_query_system/src/query/mod.rs
+++ b/compiler/rustc_query_system/src/query/mod.rs
@@ -62,18 +62,22 @@ impl QueryStackFrame {
     }
 }
 
-/// Tracks 'side effects' for a particular query.
+/// Track a 'side effects' for a particular query.
 /// This struct is saved to disk along with the query result,
 /// and loaded from disk if we mark the query as green.
 /// This allows us to 'replay' changes to global state
 /// that would otherwise only occur if we actually
 /// executed the query method.
+///
+/// Each side effect gets an unique dep node index which is added
+/// as a dependency of the query which had the effect.
 #[derive(Debug, Encodable, Decodable)]
-pub struct QuerySideEffects {
-    /// Stores any diagnostics emitted during query execution.
-    /// These diagnostics will be re-emitted if we mark
-    /// the query as green.
-    pub(super) diagnostic: DiagInner,
+pub enum QuerySideEffect {
+    /// Stores a diagnostic emitted during query execution.
+    /// This diagnostic will be re-emitted if we mark
+    /// the query as green, as that query will have the side
+    /// effect dep node as a dependency.
+    Diagnostic(DiagInner),
 }
 
 pub trait QueryContext: HasDepContext {
@@ -84,14 +88,14 @@ pub trait QueryContext: HasDepContext {
 
     fn collect_active_jobs(self) -> QueryMap;
 
-    /// Load side effects associated to the node in the previous session.
-    fn load_side_effects(
+    /// Load a side effect associated to the node in the previous session.
+    fn load_side_effect(
         self,
         prev_dep_node_index: SerializedDepNodeIndex,
-    ) -> Option<QuerySideEffects>;
+    ) -> Option<QuerySideEffect>;
 
-    /// Register diagnostics for the given node, for use in next session.
-    fn store_side_effects(self, dep_node_index: DepNodeIndex, side_effects: QuerySideEffects);
+    /// Register a side effect for the given node, for use in next session.
+    fn store_side_effect(self, dep_node_index: DepNodeIndex, side_effect: QuerySideEffect);
 
     /// Executes a job by changing the `ImplicitCtxt` to point to the
     /// new query job while it executes.
diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs
index 804b46a9bec41..8d05cd14e3820 100644
--- a/compiler/rustc_session/src/options.rs
+++ b/compiler/rustc_session/src/options.rs
@@ -2525,7 +2525,7 @@ written to standard error output)"),
         "for every macro invocation, print its name and arguments (default: no)"),
     track_diagnostics: bool = (false, parse_bool, [UNTRACKED],
         "tracks where in rustc a diagnostic was emitted"),
-    // Diagnostics are considered side-effects of a query (see `QuerySideEffects`) and are saved
+    // Diagnostics are considered side-effects of a query (see `QuerySideEffect`) and are saved
     // alongside query results and changes to translation options can affect diagnostics - so
     // translation options should be tracked.
     translate_additional_ftl: Option<PathBuf> = (None, parse_opt_pathbuf, [TRACKED],

From 9a847b1ea59090d9633a8dca75510cbe91589c00 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= <john.kare.alsaker@gmail.com>
Date: Fri, 14 Mar 2025 18:55:02 +0100
Subject: [PATCH 3/4] Add comments

---
 compiler/rustc_query_system/src/dep_graph/graph.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs
index dbc7f034a6eb0..985ca64903457 100644
--- a/compiler/rustc_query_system/src/dep_graph/graph.rs
+++ b/compiler/rustc_query_system/src/dep_graph/graph.rs
@@ -533,6 +533,8 @@ impl<D: Deps> DepGraph<D> {
         }
     }
 
+    /// This encodes a diagnostic by creating a node with an unique index and assoicating
+    /// `diagnostic` with it, for use in the next session.
     #[inline]
     pub fn record_diagnostic<Qcx: QueryContext>(&self, qcx: Qcx, diagnostic: &DiagInner) {
         if let Some(ref data) = self.data {
@@ -540,6 +542,8 @@ impl<D: Deps> DepGraph<D> {
         }
     }
 
+    /// This forces a diagnostic node green by running its side effect. `prev_index` would
+    /// refer to a node created used `encode_diagnostic` in the previous session.
     #[inline]
     pub fn force_diagnostic_node<Qcx: QueryContext>(
         &self,
@@ -673,12 +677,15 @@ 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
+    /// `diagnostic` with it, for use in the next session.
     #[inline]
     fn encode_diagnostic<Qcx: QueryContext>(
         &self,
         qcx: Qcx,
         diagnostic: &DiagInner,
     ) -> DepNodeIndex {
+        // Use `send` so we get an unique index, even though the dep node is not.
         let dep_node_index = self.current.encoder.send(
             DepNode {
                 kind: D::DEP_KIND_SIDE_EFFECT,
@@ -694,6 +701,8 @@ impl<D: Deps> DepGraphData<D> {
         dep_node_index
     }
 
+    /// This forces a diagnostic node green by running its side effect. `prev_index` would
+    /// refer to a node created used `encode_diagnostic` in the previous session.
     #[inline]
     fn force_diagnostic_node<Qcx: QueryContext>(
         &self,

From b43a29711e7ab50c1ee47a2d030273c83099b15e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= <john.kare.alsaker@gmail.com>
Date: Sat, 15 Mar 2025 03:09:09 +0100
Subject: [PATCH 4/4] Fix `record_diagnostic`

---
 compiler/rustc_query_system/src/dep_graph/graph.rs | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs
index 985ca64903457..bfd8b3207152a 100644
--- a/compiler/rustc_query_system/src/dep_graph/graph.rs
+++ b/compiler/rustc_query_system/src/dep_graph/graph.rs
@@ -538,10 +538,14 @@ impl<D: Deps> DepGraph<D> {
     #[inline]
     pub fn record_diagnostic<Qcx: QueryContext>(&self, qcx: Qcx, diagnostic: &DiagInner) {
         if let Some(ref data) = self.data {
-            self.read_index(data.encode_diagnostic(qcx, diagnostic));
+            D::read_deps(|task_deps| match task_deps {
+                TaskDepsRef::EvalAlways | TaskDepsRef::Ignore => return,
+                TaskDepsRef::Forbid | TaskDepsRef::Allow(..) => {
+                    self.read_index(data.encode_diagnostic(qcx, diagnostic));
+                }
+            })
         }
     }
-
     /// This forces a diagnostic node green by running its side effect. `prev_index` would
     /// refer to a node created used `encode_diagnostic` in the previous session.
     #[inline]