From 9012d4ecb86ff245b24cc79c6c754a501244b175 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 4 Mar 2026 18:31:16 +1100 Subject: [PATCH] Get rid of `QueryVTable::call_query_method_fn` Calling the query method is equivalent to doing a cache lookup and then calling `execute_query_fn`, so we can just do that manually instead. --- compiler/rustc_middle/src/query/plumbing.rs | 7 ----- compiler/rustc_query_impl/src/plumbing.rs | 35 ++++++++++++++------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs index e1e69a9d0b262..0e5e757b2b0a4 100644 --- a/compiler/rustc_middle/src/query/plumbing.rs +++ b/compiler/rustc_middle/src/query/plumbing.rs @@ -114,13 +114,6 @@ pub struct QueryVTable<'tcx, C: QueryCache> { pub state: QueryState<'tcx, C::Key>, pub cache: C, - /// Function pointer that calls `tcx.$query(key)` for this query and - /// discards the returned value. - /// - /// This is a weird thing to be doing, and probably not what you want. - /// It is used for loading query results from disk-cache in some cases. - pub call_query_method_fn: fn(tcx: TyCtxt<'tcx>, key: C::Key), - /// Function pointer that actually calls this query's provider. /// Also performs some associated secondary tasks; see the macro-defined /// implementation in `mod invoke_provider_fn` for more details. diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 36335a3ef9ef4..39ecb626c2773 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -19,14 +19,15 @@ use rustc_middle::query::on_disk_cache::{ }; use rustc_middle::query::plumbing::QueryVTable; use rustc_middle::query::{ - QueryCache, QueryJobId, QueryKey, QueryStackDeferred, QueryStackFrame, QueryStackFrameExtra, - erase, + QueryCache, QueryJobId, QueryKey, QueryMode, QueryStackDeferred, QueryStackFrame, + QueryStackFrameExtra, erase, }; use rustc_middle::ty::codec::TyEncoder; use rustc_middle::ty::print::with_reduced_queries; use rustc_middle::ty::tls::{self, ImplicitCtxt}; use rustc_middle::ty::{self, TyCtxt}; use rustc_serialize::{Decodable, Encodable}; +use rustc_span::DUMMY_SP; use rustc_span::def_id::LOCAL_CRATE; use crate::error::{QueryOverflow, QueryOverflowNote}; @@ -221,10 +222,27 @@ pub(crate) fn promote_from_disk_inner<'tcx, Q: GetQueryVTable<'tcx>>( dep_node.key_fingerprint ) }); - 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); + + // If the recovered key isn't eligible for cache-on-disk, then there's no + // value on disk to promote. + if !(query.will_cache_on_disk_for_key_fn)(tcx, &key) { + return; + } + + match query.cache.lookup(&key) { + // If the value is already in memory, then promotion isn't needed. + Some(_) => {} + + // "Execute" the query to load its disk-cached value into memory. + // + // We know that the key is cache-on-disk and its node is green, + // so there _must_ be a value on disk to load. + // + // FIXME(Zalathar): Is there a reasonable way to skip more of the + // query bookkeeping when doing this? + None => { + (query.execute_query_fn)(tcx, DUMMY_SP, key, QueryMode::Get); + } } } @@ -427,11 +445,6 @@ macro_rules! define_queries { state: Default::default(), cache: Default::default(), - 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. - let _ = tcx.$name(key); - }, invoke_provider_fn: self::invoke_provider_fn::__rust_begin_short_backtrace, #[cfg($cache_on_disk)]