Auto merge of #122156 - Zoxc:side-effect-dep-node, r=oli-obk

Represent diagnostic side effects as dep nodes

This changes diagnostic to be tracked as a special dep node (`SideEffect`) instead of having a list of side effects associated with each dep node. `SideEffect` is always red and when forced, it emits the diagnostic and marks itself green. Each emitted diagnostic generates a new `SideEffect` with an unique dep node index.

Some implications of this:

- Diagnostic may now be emitted more than once as they can be emitted once when the `SideEffect` gets marked green and again if the task it depends on needs to be re-executed due to another node being red. It relies on deduplicating of diagnostics to avoid that.

- Anon tasks which emits diagnostics will no longer *incorrectly* be merged with other anon tasks.

- Reusing a CGU will now emit diagnostics from the task generating it.

Author: bors

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() };

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() -> (),

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;
 }
 

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,24 +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,
-    ) -> 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.unwrap_or_default()
+        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());
     }
 
@@ -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>,

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 }
     }
 }
 

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;
@@ -26,14 +24,13 @@ use rustc_middle::ty::{self, TyCtxt};
 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};
 use rustc_serialize::{Decodable, Encodable};
 use rustc_session::Limit;
 use rustc_span::def_id::LOCAL_CRATE;
-use thin_vec::ThinVec;
 
 use crate::QueryConfigRestored;
 
@@ -93,43 +90,31 @@ impl QueryContext for QueryCtxt<'_> {
     }
 
     // Interactions with on_disk_cache
-    fn load_side_effects(self, prev_dep_node_index: SerializedDepNodeIndex) -> QuerySideEffects {
+    fn load_side_effect(
+        self,
+        prev_dep_node_index: SerializedDepNodeIndex,
+    ) -> Option<QuerySideEffect> {
         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_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)
-        }
-    }
-
-    #[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)
+            c.store_side_effect(dep_node_index, side_effect)
         }
     }
 
     /// 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
@@ -144,7 +129,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,
             };
@@ -501,7 +485,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| {
@@ -803,7 +787,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",
                 }
@@ -815,12 +799,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,

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)>,