Try mark no_hash queries as green after execution

compiler/rustc_data_structures/src/steal.rs

@@ -55,7 +55,7 @@ impl<T> Steal<T> {
 
     #[track_caller]
     pub fn steal(&self) -> T {
-        let value_ref = &mut *self.value.try_write().expect("stealing value which is locked");
+        let mut value_ref = self.value.write();
         let value = value_ref.take();
         value.expect("attempt to steal from stolen value")
     }

compiler/rustc_middle/src/dep_graph/mod.rs

@@ -12,8 +12,8 @@ pub use dep_node::{DepKind, DepNode, DepNodeExt, dep_kinds, label_strs};
 pub(crate) use dep_node::{make_compile_codegen_unit, make_compile_mono_item, make_metadata};
 pub use rustc_query_system::dep_graph::debug::{DepNodeFilter, EdgeFilter};
 pub use rustc_query_system::dep_graph::{
-    DepContext, DepGraphQuery, DepNodeIndex, Deps, SerializedDepGraph, SerializedDepNodeIndex,
-    TaskDepsRef, WorkProduct, WorkProductId, WorkProductMap, hash_result,
+    DepContext, DepGraphQuery, DepNodeIndex, Deps, MarkFrame, SerializedDepGraph,
+    SerializedDepNodeIndex, TaskDepsRef, WorkProduct, WorkProductId, WorkProductMap, hash_result,
 };
 
 pub type DepGraph = rustc_query_system::dep_graph::DepGraph<DepsType>;

compiler/rustc_query_impl/src/plumbing.rs

@@ -13,8 +13,8 @@ use rustc_hir::limit::Limit;
 use rustc_index::Idx;
 use rustc_middle::bug;
 use rustc_middle::dep_graph::{
-    self, DepContext, DepKind, DepKindStruct, DepNode, DepNodeIndex, SerializedDepNodeIndex,
-    dep_kinds,
+    self, DepContext, DepKind, DepKindStruct, DepNode, DepNodeIndex, MarkFrame,
+    SerializedDepNodeIndex, dep_kinds,
 };
 use rustc_middle::query::Key;
 use rustc_middle::query::on_disk_cache::{
@@ -489,7 +489,12 @@ where
     value
 }
 
-fn force_from_dep_node<'tcx, Q>(query: Q, tcx: TyCtxt<'tcx>, dep_node: DepNode) -> bool
+fn force_from_dep_node<'tcx, Q>(
+    query: Q,
+    tcx: TyCtxt<'tcx>,
+    dep_node: DepNode,
+    frame: &MarkFrame<'_>,
+) -> bool
 where
     Q: QueryConfig<QueryCtxt<'tcx>>,
 {
@@ -512,7 +517,7 @@ where
     );
 
     if let Some(key) = Q::Key::recover(tcx, &dep_node) {
-        force_query(query, QueryCtxt::new(tcx), key, dep_node);
+        force_query(query, QueryCtxt::new(tcx), key, dep_node, frame);
         true
     } else {
         false
@@ -540,8 +545,8 @@ where
         is_anon,
         is_eval_always,
         fingerprint_style,
-        force_from_dep_node: Some(|tcx, dep_node, _| {
-            force_from_dep_node(Q::config(tcx), tcx, dep_node)
+        force_from_dep_node: Some(|tcx, dep_node, _, frame| {
+            force_from_dep_node(Q::config(tcx), tcx, dep_node, frame)
         }),
         try_load_from_on_disk_cache: Some(|tcx, dep_node| {
             try_load_from_on_disk_cache(Q::config(tcx), tcx, dep_node)
@@ -853,7 +858,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",
                 }
@@ -865,7 +870,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: &"Red",
                 }
@@ -876,7 +881,7 @@ macro_rules! define_queries {
                     is_anon: false,
                     is_eval_always: false,
                     fingerprint_style: FingerprintStyle::Unit,
-                    force_from_dep_node: Some(|tcx, _, prev_index| {
+                    force_from_dep_node: Some(|tcx, _, prev_index, _| {
                         tcx.dep_graph.force_diagnostic_node(QueryCtxt::new(tcx), prev_index);
                         true
                     }),
@@ -890,7 +895,7 @@ macro_rules! define_queries {
                     is_anon: true,
                     is_eval_always: false,
                     fingerprint_style: FingerprintStyle::Opaque,
-                    force_from_dep_node: Some(|_, _, _| bug!("cannot force an anon node")),
+                    force_from_dep_node: Some(|_, _, _, _| bug!("cannot force an anon node")),
                     try_load_from_on_disk_cache: None,
                     name: &"AnonZeroDeps",
                 }

compiler/rustc_query_system/src/dep_graph/dep_node.rs

@@ -65,6 +65,7 @@ use rustc_hir::definitions::DefPathHash;
 use rustc_macros::{Decodable, Encodable};
 
 use super::{DepContext, FingerprintStyle, SerializedDepNodeIndex};
+use crate::dep_graph::graph::MarkFrame;
 use crate::ich::StableHashingContext;
 
 /// This serves as an index into arrays built by `make_dep_kind_array`.
@@ -276,8 +277,14 @@ 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, prev_index: SerializedDepNodeIndex) -> bool>,
+    pub force_from_dep_node: Option<
+        fn(
+            tcx: Tcx,
+            dep_node: DepNode,
+            prev_index: SerializedDepNodeIndex,
+            frame: &MarkFrame<'_>,
+        ) -> bool,
+    >,
 
     /// Invoke a query to put the on-disk cached value in memory.
     pub try_load_from_on_disk_cache: Option<fn(Tcx, DepNode)>,

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -260,7 +260,7 @@ impl<D: Deps> DepGraph<D> {
         hash_result: Option<fn(&mut StableHashingContext<'_>, &R) -> Fingerprint>,
     ) -> (R, DepNodeIndex) {
         match self.data() {
-            Some(data) => data.with_task(key, cx, arg, task, hash_result),
+            Some(data) => data.with_task(key, cx, arg, task, hash_result, None),
             None => (task(cx, arg), self.next_virtual_depnode_index()),
         }
     }
@@ -321,6 +321,7 @@ impl<D: Deps> DepGraphData<D> {
         arg: A,
         task: fn(Ctxt, A) -> R,
         hash_result: Option<fn(&mut StableHashingContext<'_>, &R) -> Fingerprint>,
+        frame: Option<&MarkFrame<'_>>,
     ) -> (R, DepNodeIndex) {
         // If the following assertion triggers, it can have two reasons:
         // 1. Something is wrong with DepNode creation, either here or
@@ -348,7 +349,8 @@ impl<D: Deps> DepGraphData<D> {
         };
 
         let dcx = cx.dep_context();
-        let dep_node_index = self.hash_result_and_alloc_node(dcx, key, edges, &result, hash_result);
+        let dep_node_index =
+            self.hash_result_and_alloc_node(*dcx, key, edges, &result, hash_result, frame, false);
 
         (result, dep_node_index)
     }
@@ -435,17 +437,20 @@ impl<D: Deps> DepGraphData<D> {
     /// Intern the new `DepNode` with the dependencies up-to-now.
     fn hash_result_and_alloc_node<Ctxt: DepContext<Deps = D>, R>(
         &self,
-        cx: &Ctxt,
+        cx: Ctxt,
         node: DepNode,
         edges: EdgesVec,
         result: &R,
         hash_result: Option<fn(&mut StableHashingContext<'_>, &R) -> Fingerprint>,
+        frame: Option<&MarkFrame<'_>>,
+        feed: bool,
     ) -> DepNodeIndex {
         let hashing_timer = cx.profiler().incr_result_hashing();
         let current_fingerprint = hash_result.map(|hash_result| {
             cx.with_stable_hashing_context(|mut hcx| hash_result(&mut hcx, result))
         });
-        let dep_node_index = self.alloc_and_color_node(node, edges, current_fingerprint);
+        let dep_node_index =
+            self.alloc_and_color_node(cx, node, edges, current_fingerprint, frame, feed);
         hashing_timer.finish_with_query_invocation_id(dep_node_index.into());
         dep_node_index
     }
@@ -599,7 +604,7 @@ impl<D: Deps> DepGraph<D> {
                 }
             });
 
-            data.hash_result_and_alloc_node(&cx, node, edges, result, hash_result)
+            data.hash_result_and_alloc_node(cx, node, edges, result, hash_result, None, true)
         } else {
             // Incremental compilation is turned off. We just execute the task
             // without tracking. We still provide a dep-node index that uniquely
@@ -717,11 +722,14 @@ impl<D: Deps> DepGraphData<D> {
         })
     }
 
-    fn alloc_and_color_node(
+    fn alloc_and_color_node<Ctxt: DepContext<Deps = D>>(
         &self,
+        cx: Ctxt,
         key: DepNode,
         edges: EdgesVec,
         fingerprint: Option<Fingerprint>,
+        frame: Option<&MarkFrame<'_>>,
+        feed: bool,
     ) -> DepNodeIndex {
         if let Some(prev_index) = self.previous.node_to_index_opt(&key) {
             // Determine the color and index of the new `DepNode`.
@@ -733,13 +741,24 @@ impl<D: Deps> DepGraphData<D> {
                 } else {
                     // This is a red node: it existed in the previous compilation, its query
                     // was re-executed, but it has a different result from before.
+                    #[cfg(debug_assertions)]
+                    if !feed && !cx.is_eval_always(key.kind) {
+                        assert_eq!(self.try_mark_previous_green(cx, prev_index, &key, frame), None);
+                    }
+
                     false
                 }
             } else {
-                // This is a red node, effectively: it existed in the previous compilation
-                // session, its query was re-executed, but it doesn't compute a result hash
-                // (i.e. it represents a `no_hash` query), so we have no way of determining
-                // whether or not the result was the same as before.
+                // This node existed in the previous compilation session. Its query was re-executed,
+                // but it doesn't compute a result hash (i.e. it represents a `no_hash` query).
+                // We can only check if node's dependencies are all green, which should be
+                // enough for us to color this node as green too.
+                if !feed && !cx.is_eval_always(key.kind) {
+                    if let Some(green) = self.try_mark_previous_green(cx, prev_index, &key, frame) {
+                        return green;
+                    }
+                }
+
                 false
             };
 
@@ -763,8 +782,6 @@ impl<D: Deps> DepGraphData<D> {
     }
 
     fn promote_node_and_deps_to_current(&self, prev_index: SerializedDepNodeIndex) -> DepNodeIndex {
-        self.current.debug_assert_not_in_new_nodes(&self.previous, prev_index);
-
         let dep_node_index = self.current.encoder.send_promoted(prev_index, &self.colors);
 
         #[cfg(debug_assertions)]
@@ -855,16 +872,16 @@ impl<D: Deps> DepGraphData<D> {
                 // in the previous compilation session too, so we can try to
                 // mark it as green by recursively marking all of its
                 // dependencies green.
-                self.try_mark_previous_green(qcx, prev_index, dep_node, None)
+                self.try_mark_previous_green(*qcx.dep_context(), prev_index, dep_node, None)
                     .map(|dep_node_index| (prev_index, dep_node_index))
             }
         }
     }
 
-    #[instrument(skip(self, qcx, parent_dep_node_index, frame), level = "debug")]
-    fn try_mark_parent_green<Qcx: QueryContext<Deps = D>>(
+    #[instrument(skip(self, cx, parent_dep_node_index, frame), level = "debug")]
+    fn try_mark_parent_green<Ctxt: DepContext<Deps = D>>(
         &self,
-        qcx: Qcx,
+        cx: Ctxt,
         parent_dep_node_index: SerializedDepNodeIndex,
         frame: &MarkFrame<'_>,
     ) -> Option<()> {
@@ -897,14 +914,14 @@ impl<D: Deps> DepGraphData<D> {
 
         // We don't know the state of this dependency. If it isn't
         // an eval_always node, let's try to mark it green recursively.
-        if !qcx.dep_context().is_eval_always(dep_dep_node.kind) {
+        if !cx.is_eval_always(dep_dep_node.kind) {
             debug!(
                 "state of dependency {:?} ({}) is unknown, trying to mark it green",
                 dep_dep_node, dep_dep_node.hash,
             );
 
             let node_index =
-                self.try_mark_previous_green(qcx, parent_dep_node_index, dep_dep_node, Some(frame));
+                self.try_mark_previous_green(cx, parent_dep_node_index, dep_dep_node, Some(frame));
 
             if node_index.is_some() {
                 debug!("managed to MARK dependency {dep_dep_node:?} as green");
@@ -914,7 +931,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, parent_dep_node_index, frame) {
+        if !cx.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;
@@ -932,7 +949,7 @@ impl<D: Deps> DepGraphData<D> {
             DepNodeColor::Unknown => {}
         }
 
-        if let None = qcx.dep_context().sess().dcx().has_errors_or_delayed_bugs() {
+        if let None = cx.sess().dcx().has_errors_or_delayed_bugs() {
             panic!("try_mark_previous_green() - Forcing the DepNode should have set its color")
         }
 
@@ -951,25 +968,25 @@ impl<D: Deps> DepGraphData<D> {
     }
 
     /// Try to mark a dep-node which existed in the previous compilation session as green.
-    #[instrument(skip(self, qcx, prev_dep_node_index, frame), level = "debug")]
-    fn try_mark_previous_green<Qcx: QueryContext<Deps = D>>(
+    #[instrument(skip(self, cx, prev_dep_node_index, frame), level = "debug")]
+    fn try_mark_previous_green<Ctxt: DepContext<Deps = D>>(
         &self,
-        qcx: Qcx,
+        cx: Ctxt,
         prev_dep_node_index: SerializedDepNodeIndex,
         dep_node: &DepNode,
         frame: Option<&MarkFrame<'_>>,
     ) -> Option<DepNodeIndex> {
         let frame = MarkFrame { index: prev_dep_node_index, parent: frame };
 
         // We never try to mark eval_always nodes as green
-        debug_assert!(!qcx.dep_context().is_eval_always(dep_node.kind));
+        debug_assert!(!cx.is_eval_always(dep_node.kind));
 
         debug_assert_eq!(self.previous.index_to_node(prev_dep_node_index), *dep_node);
 
         let prev_deps = self.previous.edge_targets_from(prev_dep_node_index);
 
         for dep_dep_node_index in prev_deps {
-            self.try_mark_parent_green(qcx, dep_dep_node_index, &frame)?;
+            self.try_mark_parent_green(cx, dep_dep_node_index, &frame)?;
         }
 
         // If we got here without hitting a `return` that means that all
@@ -1252,22 +1269,6 @@ impl<D: Deps> CurrentDepGraph<D> {
 
         dep_node_index
     }
-
-    #[inline]
-    fn debug_assert_not_in_new_nodes(
-        &self,
-        prev_graph: &SerializedDepGraph,
-        prev_index: SerializedDepNodeIndex,
-    ) {
-        if let Some(ref nodes_in_current_session) = self.nodes_in_current_session {
-            debug_assert!(
-                !nodes_in_current_session
-                    .lock()
-                    .contains_key(&prev_graph.index_to_node(prev_index)),
-                "node from previous graph present in new node collection"
-            );
-        }
-    }
 }
 
 #[derive(Debug, Clone, Copy)]

compiler/rustc_query_system/src/dep_graph/mod.rs

@@ -9,15 +9,17 @@ use std::panic;
 
 pub use dep_node::{DepKind, DepKindStruct, DepNode, DepNodeParams, WorkProductId};
 pub(crate) use graph::DepGraphData;
-pub use graph::{DepGraph, DepNodeIndex, TaskDepsRef, WorkProduct, WorkProductMap, hash_result};
+pub use graph::{
+    DepGraph, DepNodeIndex, MarkFrame, TaskDepsRef, WorkProduct, WorkProductMap, hash_result,
+};
 pub use query::DepGraphQuery;
 use rustc_data_structures::profiling::SelfProfilerRef;
 use rustc_data_structures::sync::DynSync;
 use rustc_session::Session;
 pub use serialized::{SerializedDepGraph, SerializedDepNodeIndex};
 use tracing::instrument;
 
-use self::graph::{MarkFrame, print_markframe_trace};
+use self::graph::print_markframe_trace;
 use crate::ich::StableHashingContext;
 
 pub trait DepContext: Copy {
@@ -67,7 +69,9 @@ pub trait DepContext: Copy {
     ) -> 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, prev_index))) {
+            match panic::catch_unwind(panic::AssertUnwindSafe(|| {
+                f(self, dep_node, prev_index, frame)
+            })) {
                 Err(value) => {
                     if !value.is::<rustc_errors::FatalErrorMarker>() {
                         print_markframe_trace(self.dep_graph(), frame);