From 2c057bb903a371769f3f68360ee1edbdcfca9c96 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 28 Feb 2026 17:21:56 +1100 Subject: [PATCH] Clean up `QueryVTable::hash_result` into `hash_value_fn` This commit: - 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` --- compiler/rustc_middle/src/query/inner.rs | 6 ++-- compiler/rustc_middle/src/query/plumbing.rs | 9 ++++-- compiler/rustc_query_impl/src/execution.rs | 32 ++++++++++----------- compiler/rustc_query_impl/src/plumbing.rs | 31 +++++++++++--------- 4 files changed, 42 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_middle/src/query/inner.rs b/compiler/rustc_middle/src/query/inner.rs index bd4d5bcab4a3..aa98a0267dfc 100644 --- a/compiler/rustc_middle/src/query/inner.rs +++ b/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); diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs index 55d760c55cd2..072e29aaa906 100644 --- a/compiler/rustc_middle/src/query/plumbing.rs +++ b/compiler/rustc_middle/src/query/plumbing.rs @@ -74,8 +74,6 @@ pub enum CycleErrorHandling { pub type IsLoadableFromDiskFn<'tcx, Key> = fn(tcx: TyCtxt<'tcx>, key: &Key, index: SerializedDepNodeIndex) -> bool; -pub type HashResult = Option, &V) -> Fingerprint>; - #[derive(Clone, Debug)] pub struct CycleError { /// 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>, pub is_loadable_from_disk_fn: Option>, - pub hash_result: HashResult, + + /// Function pointer that hashes this query's result values. + /// + /// For `no_hash` queries, this function pointer is None. + pub hash_value_fn: Option, &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, diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 22155e961426..5b83751ae05f 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -354,7 +354,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) @@ -366,18 +366,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 { @@ -389,17 +389,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. @@ -420,7 +420,7 @@ 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()); @@ -428,14 +428,14 @@ fn execute_job_non_incr<'tcx, C: QueryCache>( // 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)] @@ -491,7 +491,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, ) }); @@ -542,7 +542,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, ); } @@ -589,7 +589,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, ); diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 8920f8dba38d..272185845423 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -210,19 +210,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 { @@ -606,7 +600,16 @@ pub(crate) fn make_query_vtable<'tcx>(incremental: bool) 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>| { + let value = erase::restore_val(*erased_value); + rustc_middle::dep_graph::hash_result(hcx, &value) + }) + } + ), format_value: |value| format!("{:?}", erase::restore_val::>(*value)), description_fn: $crate::queries::_description_fns::$name, execute_query_fn: if incremental {