Auto merge of #151853 - Zalathar:feed-vtable, r=oli-obk

Use the query vtable in `query_feed` plumbing

The `query_feed` function needs to be able to do two important things with (erased) query values: hash them, and debug-print them.

Both of those are things that the query's vtable already knows how to do. So by passing in a vtable to `query_feed`, we can give it a nicer signature, avoid having to unerase values in the function itself, and clean up some caller-side code as well.

Author: bors

compiler/rustc_middle/src/query/inner.rs

@@ -1,16 +1,13 @@
 //! Helper functions that serve as the immediate implementation of
 //! `tcx.$query(..)` and its variations.
 
-use std::fmt::Debug;
-
-use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_query_system::dep_graph::{DepKind, DepNodeParams};
-use rustc_query_system::ich::StableHashingContext;
 use rustc_query_system::query::{QueryCache, QueryMode, try_get_cached};
 use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span};
 
 use crate::dep_graph;
 use crate::query::erase::{self, Erasable, Erased};
+use crate::query::plumbing::QueryVTable;
 use crate::ty::TyCtxt;
 
 /// Shared implementation of `tcx.$query(..)` and `tcx.at(span).$query(..)`
@@ -80,35 +77,38 @@ where
 }
 
 /// Common implementation of query feeding, used by `define_feedable!`.
-pub(crate) fn query_feed<'tcx, Cache, Value>(
+pub(crate) fn query_feed<'tcx, Cache>(
     tcx: TyCtxt<'tcx>,
     dep_kind: DepKind,
-    hasher: Option<fn(&mut StableHashingContext<'_>, &Value) -> Fingerprint>,
+    query_vtable: &QueryVTable<'tcx, Cache>,
     cache: &Cache,
     key: Cache::Key,
-    erased: Erased<Value>,
+    value: Cache::Value,
 ) where
-    Cache: QueryCache<Value = Erased<Value>>,
+    Cache: QueryCache,
     Cache::Key: DepNodeParams<TyCtxt<'tcx>>,
-    Value: Erasable + Debug,
 {
-    let value = erase::restore_val::<Value>(erased);
+    let format_value = query_vtable.format_value;
 
+    // Check whether the in-memory cache already has a value for this key.
     match try_get_cached(tcx, cache, &key) {
         Some(old) => {
-            let old = erase::restore_val::<Value>(old);
-            if let Some(hasher) = hasher {
-                let (value_hash, old_hash): (Fingerprint, Fingerprint) = tcx
-                    .with_stable_hashing_context(|mut hcx| {
-                        (hasher(&mut hcx, &value), hasher(&mut hcx, &old))
-                    });
+            // The query already has a cached value for this key.
+            // That's OK if both values are the same, i.e. they have the same hash,
+            // so now we check their hashes.
+            if let Some(hasher_fn) = query_vtable.hash_result {
+                let (old_hash, value_hash) = tcx.with_stable_hashing_context(|ref mut hcx| {
+                    (hasher_fn(hcx, &old), hasher_fn(hcx, &value))
+                });
                 if old_hash != value_hash {
                     // We have an inconsistency. This can happen if one of the two
                     // results is tainted by errors. In this case, delay a bug to
                     // ensure compilation is doomed, and keep the `old` value.
                     tcx.dcx().delayed_bug(format!(
                         "Trying to feed an already recorded value for query {dep_kind:?} key={key:?}:\n\
-                        old value: {old:?}\nnew value: {value:?}",
+                        old value: {old}\nnew value: {value}",
+                        old = format_value(&old),
+                        value = format_value(&value),
                     ));
                 }
             } else {
@@ -117,14 +117,24 @@ pub(crate) fn query_feed<'tcx, Cache, Value>(
                 // the query should not be marked `no_hash`.
                 bug!(
                     "Trying to feed an already recorded value for query {dep_kind:?} key={key:?}:\n\
-                    old value: {old:?}\nnew value: {value:?}",
+                    old value: {old}\nnew value: {value}",
+                    old = format_value(&old),
+                    value = format_value(&value),
                 )
             }
         }
         None => {
+            // There is no cached value for this key, so feed the query by
+            // adding the provided value to the cache.
             let dep_node = dep_graph::DepNode::construct(tcx, dep_kind, &key);
-            let dep_node_index = tcx.dep_graph.with_feed_task(dep_node, tcx, &value, hasher);
-            cache.complete(key, erased, dep_node_index);
+            let dep_node_index = tcx.dep_graph.with_feed_task(
+                dep_node,
+                tcx,
+                &value,
+                query_vtable.hash_result,
+                query_vtable.format_value,
+            );
+            cache.complete(key, value, dep_node_index);
         }
     }
 }

compiler/rustc_middle/src/query/mod.rs

@@ -87,7 +87,6 @@ use rustc_hir::{Crate, ItemLocalId, ItemLocalMap, PreciseCapturingArgKind, Trait
 use rustc_index::IndexVec;
 use rustc_lint_defs::LintId;
 use rustc_macros::rustc_queries;
-use rustc_query_system::ich::StableHashingContext;
 use rustc_query_system::query::{QueryMode, QueryState};
 use rustc_session::Limits;
 use rustc_session::config::{EntryFnType, OptLevel, OutputFilenames, SymbolManglingVersion};

compiler/rustc_middle/src/query/plumbing.rs

@@ -497,18 +497,6 @@ macro_rules! define_callbacks {
     };
 }
 
-macro_rules! hash_result {
-    ([]) => {{
-        Some(dep_graph::hash_result)
-    }};
-    ([(no_hash) $($rest:tt)*]) => {{
-        None
-    }};
-    ([$other:tt $($modifiers:tt)*]) => {
-        hash_result!([$($modifiers)*])
-    };
-}
-
 macro_rules! define_feedable {
     ($($(#[$attr:meta])* [$($modifiers:tt)*] fn $name:ident($($K:tt)*) -> $V:ty,)*) => {
         $(impl<'tcx, K: IntoQueryParam<$($K)*> + Copy> TyCtxtFeed<'tcx, K> {
@@ -518,19 +506,17 @@ macro_rules! define_feedable {
                 let key = self.key().into_query_param();
 
                 let tcx = self.tcx;
-                let erased = queries::$name::provided_to_erased(tcx, value);
-                let cache = &tcx.query_system.caches.$name;
+                let erased_value = queries::$name::provided_to_erased(tcx, value);
 
                 let dep_kind: dep_graph::DepKind = dep_graph::dep_kinds::$name;
-                let hasher: Option<fn(&mut StableHashingContext<'_>, &_) -> _> = hash_result!([$($modifiers)*]);
 
                 $crate::query::inner::query_feed(
                     tcx,
                     dep_kind,
-                    hasher,
-                    cache,
+                    &tcx.query_system.query_vtables.$name,
+                    &tcx.query_system.caches.$name,
                     key,
-                    erased,
+                    erased_value,
                 );
             }
         })*

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -561,12 +561,13 @@ impl<D: Deps> DepGraph<D> {
     /// FIXME: If the code is changed enough for this node to be marked before requiring the
     /// caller's node, we suppose that those changes will be enough to mark this node red and
     /// force a recomputation using the "normal" way.
-    pub fn with_feed_task<Ctxt: DepContext<Deps = D>, R: Debug>(
+    pub fn with_feed_task<Ctxt: DepContext<Deps = D>, R>(
         &self,
         node: DepNode,
         cx: Ctxt,
         result: &R,
         hash_result: Option<fn(&mut StableHashingContext<'_>, &R) -> Fingerprint>,
+        format_value_fn: fn(&R) -> String,
     ) -> DepNodeIndex {
         if let Some(data) = self.data.as_ref() {
             // The caller query has more dependencies than the node we are creating. We may
@@ -584,7 +585,7 @@ impl<D: Deps> DepGraph<D> {
                         result,
                         prev_index,
                         hash_result,
-                        |value| format!("{value:?}"),
+                        format_value_fn,
                     );
 
                     #[cfg(debug_assertions)]