From d8638505743b7f36c4932c510cd64fdb1c15f72b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 3 Mar 2026 14:11:49 +1100 Subject: [PATCH 1/2] Streamline cache-related query functions. There are three query vtable functions related to the `cache_on_disk_if` modifier: - `will_cache_on_disk_for_key_fn` - `try_load_from_disk_fn` - `is_loadable_from_disk_fn` These are all function ptrs within an `Option`. They each have a wrapper that returns `false`/`None` if the function ptr is missing. This commit removes the `Option` wrappers. In the `None` case we now set the function ptr to a trivial closure that returns `false`/`None`. The commit also removes some typedefs that each have a single use. All this is a bit simpler, with less indirection. Note that `QueryVTable::hash_value_fn` is still an `Option`. Unlike the above three functions, which have trivial behaviour in the `None` case, `hash_value_fn` is used in ways where the `Some`/`None` distinction is more meaningful. --- compiler/rustc_middle/src/query/plumbing.rs | 56 ++++----------------- compiler/rustc_query_impl/src/execution.rs | 14 +++--- compiler/rustc_query_impl/src/plumbing.rs | 28 +++++------ 3 files changed, 30 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs index 815c8a1baab61..65078a2a68b63 100644 --- a/compiler/rustc_middle/src/query/plumbing.rs +++ b/compiler/rustc_middle/src/query/plumbing.rs @@ -114,7 +114,6 @@ pub struct QueryVTable<'tcx, C: QueryCache> { pub cycle_error_handling: CycleErrorHandling, pub state: QueryState<'tcx, C::Key>, pub cache: C, - pub will_cache_on_disk_for_key_fn: Option, key: &C::Key) -> bool>, /// Function pointer that calls `tcx.$query(key)` for this query and /// discards the returned value. @@ -130,17 +129,17 @@ pub struct QueryVTable<'tcx, C: QueryCache> { /// This should be the only code that calls the provider function. pub invoke_provider_fn: fn(tcx: TyCtxt<'tcx>, key: C::Key) -> C::Value, - pub try_load_from_disk_fn: Option< - fn( - tcx: TyCtxt<'tcx>, - key: &C::Key, - prev_index: SerializedDepNodeIndex, - index: DepNodeIndex, - ) -> Option, - >, + pub will_cache_on_disk_for_key_fn: fn(tcx: TyCtxt<'tcx>, key: &C::Key) -> bool, + + pub try_load_from_disk_fn: fn( + tcx: TyCtxt<'tcx>, + key: &C::Key, + prev_index: SerializedDepNodeIndex, + index: DepNodeIndex, + ) -> Option, pub is_loadable_from_disk_fn: - Option, key: &C::Key, index: SerializedDepNodeIndex) -> bool>, + fn(tcx: TyCtxt<'tcx>, key: &C::Key, index: SerializedDepNodeIndex) -> bool, /// Function pointer that hashes this query's result values. /// @@ -182,43 +181,6 @@ impl<'tcx, C: QueryCache> fmt::Debug for QueryVTable<'tcx, C> { } impl<'tcx, C: QueryCache> QueryVTable<'tcx, C> { - #[inline(always)] - pub fn will_cache_on_disk_for_key(&self, tcx: TyCtxt<'tcx>, key: &C::Key) -> bool { - self.will_cache_on_disk_for_key_fn.map_or(false, |f| f(tcx, key)) - } - - #[inline(always)] - pub fn try_load_from_disk( - &self, - tcx: TyCtxt<'tcx>, - key: &C::Key, - prev_index: SerializedDepNodeIndex, - index: DepNodeIndex, - ) -> Option { - // `?` will return None immediately for queries that never cache to disk. - self.try_load_from_disk_fn?(tcx, key, prev_index, index) - } - - #[inline] - pub fn is_loadable_from_disk( - &self, - tcx: TyCtxt<'tcx>, - key: &C::Key, - index: SerializedDepNodeIndex, - ) -> bool { - self.is_loadable_from_disk_fn.map_or(false, |f| f(tcx, key, index)) - } - - /// Synthesize an error value to let compilation continue after a cycle. - pub fn value_from_cycle_error( - &self, - tcx: TyCtxt<'tcx>, - cycle_error: CycleError, - guar: ErrorGuaranteed, - ) -> C::Value { - (self.value_from_cycle_error)(tcx, cycle_error, guar) - } - pub fn construct_dep_node(&self, tcx: TyCtxt<'tcx>, key: &C::Key) -> DepNode { DepNode::construct(tcx, self.dep_kind, key) } diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 323d40fdbaabb..5336b22514f2f 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -129,7 +129,7 @@ fn mk_cycle<'tcx, C: QueryCache>( match query.cycle_error_handling { CycleErrorHandling::Error => { let guar = error.emit(); - query.value_from_cycle_error(tcx, cycle_error, guar) + (query.value_from_cycle_error)(tcx, cycle_error, guar) } CycleErrorHandling::Fatal => { let guar = error.emit(); @@ -137,7 +137,7 @@ fn mk_cycle<'tcx, C: QueryCache>( } CycleErrorHandling::DelayBug => { let guar = error.delay_as_bug(); - query.value_from_cycle_error(tcx, cycle_error, guar) + (query.value_from_cycle_error)(tcx, cycle_error, guar) } CycleErrorHandling::Stash => { let guar = if let Some(root) = cycle_error.cycle.first() @@ -147,7 +147,7 @@ fn mk_cycle<'tcx, C: QueryCache>( } else { error.emit() }; - query.value_from_cycle_error(tcx, cycle_error, guar) + (query.value_from_cycle_error)(tcx, cycle_error, guar) } } } @@ -522,7 +522,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>( // First we try to load the result from the on-disk cache. // Some things are never cached on disk. - if let Some(value) = query.try_load_from_disk(tcx, key, prev_index, dep_node_index) { + if let Some(value) = (query.try_load_from_disk_fn)(tcx, key, prev_index, dep_node_index) { if std::intrinsics::unlikely(tcx.sess.opts.unstable_opts.query_dep_graph) { dep_graph_data.mark_debug_loaded_from_disk(*dep_node) } @@ -555,7 +555,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>( // We always expect to find a cached result for things that // can be forced from `DepNode`. debug_assert!( - !query.will_cache_on_disk_for_key(tcx, key) + !(query.will_cache_on_disk_for_key_fn)(tcx, key) || !tcx.key_fingerprint_style(dep_node.kind).is_maybe_recoverable(), "missing on-disk cache entry for {dep_node:?}" ); @@ -563,7 +563,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>( // Sanity check for the logic in `ensure`: if the node is green and the result loadable, // we should actually be able to load it. debug_assert!( - !query.is_loadable_from_disk(tcx, key, prev_index), + !(query.is_loadable_from_disk_fn)(tcx, key, prev_index), "missing on-disk cache entry for loadable {dep_node:?}" ); @@ -660,7 +660,7 @@ fn check_if_ensure_can_skip_execution<'tcx, C: QueryCache>( // In ensure-done mode, we can only skip execution for this key if // there's a disk-cached value available to load later if needed, // which guarantees the query provider will never run for this key. - let is_loadable = query.is_loadable_from_disk(tcx, key, serialized_dep_node_index); + let is_loadable = (query.is_loadable_from_disk_fn)(tcx, key, serialized_dep_node_index); EnsureCanSkip { skip_execution: is_loadable, dep_node: Some(dep_node) } } } diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 425ca28910073..36335a3ef9ef4 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -168,7 +168,7 @@ pub(crate) fn encode_query_results<'a, 'tcx, C, V>( assert!(all_inactive(&query.state)); query.cache.iter(&mut |key, value, dep_node| { - if query.will_cache_on_disk_for_key(tcx, key) { + if (query.will_cache_on_disk_for_key_fn)(tcx, key) { let dep_node = SerializedDepNodeIndex::new(dep_node.index()); // Record position of the cache entry. @@ -221,7 +221,7 @@ pub(crate) fn promote_from_disk_inner<'tcx, Q: GetQueryVTable<'tcx>>( dep_node.key_fingerprint ) }); - if query.will_cache_on_disk_for_key(tcx, &key) { + if (query.will_cache_on_disk_for_key_fn)(tcx, &key) { // Call `tcx.$query(key)` for its side-effect of loading the disk-cached // value into memory. (query.call_query_method_fn)(tcx, key); @@ -427,12 +427,6 @@ macro_rules! define_queries { state: Default::default(), cache: Default::default(), - #[cfg($cache_on_disk)] - will_cache_on_disk_for_key_fn: - Some(rustc_middle::queries::_cache_on_disk_if_fns::$name), - #[cfg(not($cache_on_disk))] - will_cache_on_disk_for_key_fn: None, - call_query_method_fn: |tcx, key| { // Call the query method for its side-effect of loading a value // from disk-cache; the caller doesn't need the value. @@ -441,7 +435,13 @@ macro_rules! define_queries { invoke_provider_fn: self::invoke_provider_fn::__rust_begin_short_backtrace, #[cfg($cache_on_disk)] - try_load_from_disk_fn: Some(|tcx, key, prev_index, index| { + will_cache_on_disk_for_key_fn: + rustc_middle::queries::_cache_on_disk_if_fns::$name, + #[cfg(not($cache_on_disk))] + will_cache_on_disk_for_key_fn: |_, _| false, + + #[cfg($cache_on_disk)] + try_load_from_disk_fn: |tcx, key, prev_index, index| { // Check the `cache_on_disk_if` condition for this key. if !rustc_middle::queries::_cache_on_disk_if_fns::$name(tcx, key) { return None; @@ -452,17 +452,17 @@ macro_rules! define_queries { // Arena-alloc the value if appropriate, and erase it. Some(queries::$name::provided_to_erased(tcx, value)) - }), + }, #[cfg(not($cache_on_disk))] - try_load_from_disk_fn: None, + try_load_from_disk_fn: |_tcx, _key, _prev_index, _index| None, #[cfg($cache_on_disk)] - is_loadable_from_disk_fn: Some(|tcx, key, index| -> bool { + is_loadable_from_disk_fn: |tcx, key, index| -> bool { rustc_middle::queries::_cache_on_disk_if_fns::$name(tcx, key) && $crate::plumbing::loadable_from_disk(tcx, index) - }), + }, #[cfg(not($cache_on_disk))] - is_loadable_from_disk_fn: None, + is_loadable_from_disk_fn: |_tcx, _key, _index| false, value_from_cycle_error: |tcx, cycle, guar| { let result: queries::$name::Value<'tcx> = From d08f97d739ab999d486d318f81efa4987ee82e4c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 3 Mar 2026 14:17:06 +1100 Subject: [PATCH 2/2] Remove `QueryVTable::construct_dep_node`. It's just a wrapper for `DepNode::construct` and it only has three uses. Better to just have one way of doing things. --- compiler/rustc_middle/src/query/plumbing.rs | 8 +------- compiler/rustc_query_impl/src/execution.rs | 8 +++++--- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs index 65078a2a68b63..6193de8f77e44 100644 --- a/compiler/rustc_middle/src/query/plumbing.rs +++ b/compiler/rustc_middle/src/query/plumbing.rs @@ -11,7 +11,7 @@ use rustc_macros::HashStable; use rustc_span::{ErrorGuaranteed, Span}; pub use sealed::IntoQueryParam; -use crate::dep_graph::{DepKind, DepNode, DepNodeIndex, SerializedDepNodeIndex}; +use crate::dep_graph::{DepKind, DepNodeIndex, SerializedDepNodeIndex}; use crate::ich::StableHashingContext; use crate::queries::{ExternProviders, Providers, QueryArenas, QueryVTables}; use crate::query::on_disk_cache::OnDiskCache; @@ -180,12 +180,6 @@ impl<'tcx, C: QueryCache> fmt::Debug for QueryVTable<'tcx, C> { } } -impl<'tcx, C: QueryCache> QueryVTable<'tcx, C> { - pub fn construct_dep_node(&self, tcx: TyCtxt<'tcx>, key: &C::Key) -> DepNode { - DepNode::construct(tcx, self.dep_kind, key) - } -} - pub struct QuerySystem<'tcx> { pub arenas: WorkerLocal>, pub query_vtables: QueryVTables<'tcx>, diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 5336b22514f2f..3dfc3ca2ed3cb 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -453,7 +453,8 @@ fn execute_job_incr<'tcx, C: QueryCache>( if !query.anon && !query.eval_always { // `to_dep_node` is expensive for some `DepKind`s. - let dep_node = dep_node_opt.get_or_insert_with(|| query.construct_dep_node(tcx, &key)); + let dep_node = + dep_node_opt.get_or_insert_with(|| DepNode::construct(tcx, query.dep_kind, &key)); // The diagnostics for this query will be promoted to the current session during // `try_mark_green()`, so we can ignore them here. @@ -485,7 +486,8 @@ fn execute_job_incr<'tcx, C: QueryCache>( } // `to_dep_node` is expensive for some `DepKind`s. - let dep_node = dep_node_opt.unwrap_or_else(|| query.construct_dep_node(tcx, &key)); + let dep_node = + dep_node_opt.unwrap_or_else(|| DepNode::construct(tcx, query.dep_kind, &key)); // Call the query provider. dep_graph_data.with_task( @@ -629,7 +631,7 @@ fn check_if_ensure_can_skip_execution<'tcx, C: QueryCache>( // Ensuring an anonymous query makes no sense assert!(!query.anon); - let dep_node = query.construct_dep_node(tcx, key); + let dep_node = DepNode::construct(tcx, query.dep_kind, key); let dep_graph = &tcx.dep_graph; let serialized_dep_node_index = match dep_graph.try_mark_green(tcx, &dep_node) {