Expand hash check.

Author: cjgillot

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -513,15 +513,18 @@ impl<K: DepKind> DepGraph<K> {
         hash_result: fn(&mut StableHashingContext<'_>, &R) -> Fingerprint,
     ) -> DepNodeIndex {
         if let Some(data) = self.data.as_ref() {
+            // The caller query has more dependencies than the node we are creating.  We may
+            // encounter a case where this created node is marked as green, but the caller query is
+            // subsequently marked as red or recomputed.  In this case, we will end up feeding a
+            // value to an existing node.
+            //
+            // For sanity, we still check that the loaded stable hash and the new one match.
             if let Some(dep_node_index) = self.dep_node_index_of_opt(&node) {
+                let _current_fingerprint =
+                    crate::query::incremental_verify_ich(cx, result, &node, Some(hash_result));
+
                 #[cfg(debug_assertions)]
-                {
-                    let hashing_timer = cx.profiler().incr_result_hashing();
-                    let current_fingerprint =
-                        cx.with_stable_hashing_context(|mut hcx| hash_result(&mut hcx, result));
-                    hashing_timer.finish_with_query_invocation_id(dep_node_index.into());
-                    data.current.record_edge(dep_node_index, node, current_fingerprint);
-                }
+                data.current.record_edge(dep_node_index, node, _current_fingerprint);
 
                 return dep_node_index;
             }

compiler/rustc_query_system/src/query/plumbing.rs

@@ -3,6 +3,7 @@
 //! manage the caches, and so forth.
 
 use crate::dep_graph::{DepContext, DepNode, DepNodeIndex, DepNodeParams};
+use crate::ich::StableHashingContext;
 use crate::query::caches::QueryCache;
 use crate::query::config::QueryVTable;
 use crate::query::job::{report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobInfo};
@@ -525,7 +526,7 @@ where
             if std::intrinsics::unlikely(
                 try_verify || qcx.dep_context().sess().opts.unstable_opts.incremental_verify_ich,
             ) {
-                incremental_verify_ich(*qcx.dep_context(), &result, dep_node, query);
+                incremental_verify_ich(*qcx.dep_context(), &result, dep_node, query.hash_result);
             }
 
             return Some((result, dep_node_index));
@@ -558,39 +559,42 @@ where
     //
     // See issue #82920 for an example of a miscompilation that would get turned into
     // an ICE by this check
-    incremental_verify_ich(*qcx.dep_context(), &result, dep_node, query);
+    incremental_verify_ich(*qcx.dep_context(), &result, dep_node, query.hash_result);
 
     Some((result, dep_node_index))
 }
 
-#[instrument(skip(qcx, result, query), level = "debug")]
-fn incremental_verify_ich<Qcx, K, V: Debug>(
-    qcx: Qcx::DepContext,
+#[instrument(skip(tcx, result, hash_result), level = "debug")]
+pub(crate) fn incremental_verify_ich<Tcx, V: Debug>(
+    tcx: Tcx,
     result: &V,
-    dep_node: &DepNode<Qcx::DepKind>,
-    query: &QueryVTable<Qcx, K, V>,
-) where
-    Qcx: QueryContext,
+    dep_node: &DepNode<Tcx::DepKind>,
+    hash_result: Option<fn(&mut StableHashingContext<'_>, &V) -> Fingerprint>,
+) -> Fingerprint
+where
+    Tcx: DepContext,
 {
     assert!(
-        qcx.dep_graph().is_green(dep_node),
+        tcx.dep_graph().is_green(dep_node),
         "fingerprint for green query instance not loaded from cache: {:?}",
         dep_node,
     );
 
-    let new_hash = query.hash_result.map_or(Fingerprint::ZERO, |f| {
-        qcx.with_stable_hashing_context(|mut hcx| f(&mut hcx, result))
+    let new_hash = hash_result.map_or(Fingerprint::ZERO, |f| {
+        tcx.with_stable_hashing_context(|mut hcx| f(&mut hcx, result))
     });
 
-    let old_hash = qcx.dep_graph().prev_fingerprint_of(dep_node);
+    let old_hash = tcx.dep_graph().prev_fingerprint_of(dep_node);
 
     if Some(new_hash) != old_hash {
         incremental_verify_ich_failed(
-            qcx.sess(),
+            tcx.sess(),
             DebugArg::from(&dep_node),
             DebugArg::from(&result),
         );
     }
+
+    new_hash
 }
 
 // This DebugArg business is largely a mirror of std::fmt::ArgumentV1, which is