Rollup merge of #153209 - Zalathar:hash-value-fn, r=nnethercote

Clean up `QueryVTable::hash_result` into `hash_value_fn`

This PR:
- Renames the query vtable field `hash_result` to `hash_value_fn`
- Removes the unhelpful `HashResult` type alias, which was hiding an important layer of `Option`
- Replaces the cryptic `hash_result!` helper macro with a more straightforward `if_no_hash!` helper, in line with other modifier-checking macros
- Renames a few identifiers to refer to a query's return value as `value`

There should be no change to compiler behaviour.

r? nnethercote (or compiler)

Author: matthiaskrgr

compiler/rustc_middle/src/query/inner.rs

@@ -116,9 +116,9 @@ pub(crate) fn query_feed<'tcx, C>(
             // 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 {
+            if let Some(hash_value_fn) = query_vtable.hash_value_fn {
                 let (old_hash, value_hash) = tcx.with_stable_hashing_context(|ref mut hcx| {
-                    (hasher_fn(hcx, &old), hasher_fn(hcx, &value))
+                    (hash_value_fn(hcx, &old), hash_value_fn(hcx, &value))
                 });
                 if old_hash != value_hash {
                     // We have an inconsistency. This can happen if one of the two
@@ -151,7 +151,7 @@ pub(crate) fn query_feed<'tcx, C>(
                 dep_node,
                 tcx,
                 &value,
-                query_vtable.hash_result,
+                query_vtable.hash_value_fn,
                 query_vtable.format_value,
             );
             query_vtable.cache.complete(key, value, dep_node_index);

compiler/rustc_middle/src/query/plumbing.rs

@@ -74,8 +74,6 @@ pub type TryLoadFromDiskFn<'tcx, Key, Value> = fn(
 pub type IsLoadableFromDiskFn<'tcx, Key> =
     fn(tcx: TyCtxt<'tcx>, key: &Key, index: SerializedDepNodeIndex) -> bool;
 
-pub type HashResult<V> = Option<fn(&mut StableHashingContext<'_>, &V) -> Fingerprint>;
-
 #[derive(Clone, Debug)]
 pub struct CycleError<I = QueryStackFrameExtra> {
     /// The query and related span that uses the cycle.
@@ -146,7 +144,12 @@ pub struct QueryVTable<'tcx, C: QueryCache> {
 
     pub try_load_from_disk_fn: Option<TryLoadFromDiskFn<'tcx, C::Key, C::Value>>,
     pub is_loadable_from_disk_fn: Option<IsLoadableFromDiskFn<'tcx, C::Key>>,
-    pub hash_result: HashResult<C::Value>,
+
+    /// Function pointer that hashes this query's result values.
+    ///
+    /// For `no_hash` queries, this function pointer is None.
+    pub hash_value_fn: Option<fn(&mut StableHashingContext<'_>, &C::Value) -> Fingerprint>,
+
     pub value_from_cycle_error:
         fn(tcx: TyCtxt<'tcx>, cycle_error: &CycleError, guar: ErrorGuaranteed) -> C::Value,
     pub format_value: fn(&C::Value) -> String,

compiler/rustc_query_impl/src/execution.rs

@@ -372,7 +372,7 @@ fn execute_job<'tcx, C: QueryCache, const INCR: bool>(
     debug_assert_eq!(tcx.dep_graph.is_fully_enabled(), INCR);
 
     // Delegate to another function to actually execute the query job.
-    let (result, dep_node_index) = if INCR {
+    let (value, dep_node_index) = if INCR {
         execute_job_incr(query, tcx, key, dep_node, id)
     } else {
         execute_job_non_incr(query, tcx, key, id)
@@ -384,18 +384,18 @@ fn execute_job<'tcx, C: QueryCache, const INCR: bool>(
         // This can't happen, as query feeding adds the very dependencies to the fed query
         // as its feeding query had. So if the fed query is red, so is its feeder, which will
         // get evaluated first, and re-feed the query.
-        if let Some((cached_result, _)) = cache.lookup(&key) {
-            let Some(hasher) = query.hash_result else {
+        if let Some((cached_value, _)) = cache.lookup(&key) {
+            let Some(hash_value_fn) = query.hash_value_fn else {
                 panic!(
                     "no_hash fed query later has its value computed.\n\
                     Remove `no_hash` modifier to allow recomputation.\n\
                     The already cached value: {}",
-                    (query.format_value)(&cached_result)
+                    (query.format_value)(&cached_value)
                 );
             };
 
             let (old_hash, new_hash) = tcx.with_stable_hashing_context(|mut hcx| {
-                (hasher(&mut hcx, &cached_result), hasher(&mut hcx, &result))
+                (hash_value_fn(&mut hcx, &cached_value), hash_value_fn(&mut hcx, &value))
             });
             let formatter = query.format_value;
             if old_hash != new_hash {
@@ -407,17 +407,17 @@ fn execute_job<'tcx, C: QueryCache, const INCR: bool>(
                         computed={:#?}\nfed={:#?}",
                     query.dep_kind,
                     key,
-                    formatter(&result),
-                    formatter(&cached_result),
+                    formatter(&value),
+                    formatter(&cached_value),
                 );
             }
         }
     }
 
     // Tell the guard to perform completion bookkeeping, and also to not poison the query.
-    job_guard.complete(cache, result, dep_node_index);
+    job_guard.complete(cache, value, dep_node_index);
 
-    (result, Some(dep_node_index))
+    (value, Some(dep_node_index))
 }
 
 // Fast path for when incr. comp. is off.
@@ -438,22 +438,22 @@ fn execute_job_non_incr<'tcx, C: QueryCache>(
 
     let prof_timer = tcx.prof.query_provider();
     // Call the query provider.
-    let result =
+    let value =
         start_query(tcx, job_id, query.depth_limit, || (query.invoke_provider_fn)(tcx, key));
     let dep_node_index = tcx.dep_graph.next_virtual_depnode_index();
     prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 
     // Similarly, fingerprint the result to assert that
     // it doesn't have anything not considered hashable.
     if cfg!(debug_assertions)
-        && let Some(hash_result) = query.hash_result
+        && let Some(hash_value_fn) = query.hash_value_fn
     {
         tcx.with_stable_hashing_context(|mut hcx| {
-            hash_result(&mut hcx, &result);
+            hash_value_fn(&mut hcx, &value);
         });
     }
 
-    (result, dep_node_index)
+    (value, dep_node_index)
 }
 
 #[inline(always)]
@@ -509,7 +509,7 @@ fn execute_job_incr<'tcx, C: QueryCache>(
             tcx,
             (query, key),
             |tcx, (query, key)| (query.invoke_provider_fn)(tcx, key),
-            query.hash_result,
+            query.hash_value_fn,
         )
     });
 
@@ -560,7 +560,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>(
                 dep_graph_data,
                 &value,
                 prev_index,
-                query.hash_result,
+                query.hash_value_fn,
                 query.format_value,
             );
         }
@@ -607,7 +607,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>(
         dep_graph_data,
         &value,
         prev_index,
-        query.hash_result,
+        query.hash_value_fn,
         query.format_value,
     );
 

compiler/rustc_query_impl/src/plumbing.rs

@@ -165,19 +165,13 @@ macro_rules! is_feedable {
     };
 }
 
-macro_rules! hash_result {
-    ([][$V:ty]) => {{
-        Some(|hcx, result| {
-            let result = rustc_middle::query::erase::restore_val::<$V>(*result);
-            rustc_middle::dep_graph::hash_result(hcx, &result)
-        })
-    }};
-    ([(no_hash) $($rest:tt)*][$V:ty]) => {{
-        None
-    }};
-    ([$other:tt $($modifiers:tt)*][$($args:tt)*]) => {
-        hash_result!([$($modifiers)*][$($args)*])
-    };
+/// Expands to `$yes` if the `no_hash` modifier is present, or `$no` otherwise.
+macro_rules! if_no_hash {
+    ([] $yes:tt $no:tt) => { $no };
+    ([(no_hash) $($modifiers:tt)*] $yes:tt $no:tt) => { $yes };
+    ([$other:tt $($modifiers:tt)*] $yes:tt $no:tt) => {
+        if_no_hash!([$($modifiers)*] $yes $no)
+    }
 }
 
 macro_rules! call_provider {
@@ -560,7 +554,16 @@ macro_rules! define_queries {
                         let result: queries::$name::Value<'tcx> = Value::from_cycle_error(tcx, cycle, guar);
                         erase::erase_val(result)
                     },
-                    hash_result: hash_result!([$($modifiers)*][queries::$name::Value<'tcx>]),
+                    hash_value_fn: if_no_hash!(
+                        [$($modifiers)*]
+                        None
+                        {
+                            Some(|hcx, erased_value: &erase::Erased<queries::$name::Value<'tcx>>| {
+                                let value = erase::restore_val(*erased_value);
+                                rustc_middle::dep_graph::hash_result(hcx, &value)
+                            })
+                        }
+                    ),
                     format_value: |value| format!("{:?}", erase::restore_val::<queries::$name::Value<'tcx>>(*value)),
                     description_fn: $crate::queries::_description_fns::$name,
                     execute_query_fn: if incremental {