Rollup merge of #147393 - Zalathar:feed, r=cjgillot

matthiaskrgr · web-flow · commit 0765b43b37bc · 2025-10-12T10:13:16.000+02:00
Extract most code from `define_feedable!`

This PR extracts most of the non-trivial code from the `define_feedable!` macro (which defines the `TyCtxtFeed::$query` methods), and moves it to a helper function `query_feed_inner` written in ordinary non-macro code.

Doing so should make that code easier to read and modify, because it now gets proper IDE support and has explicit trait bounds.

---

There should be no change in compiler behaviour.

I've structured the commits so that the actual extraction part is mostly just whitespace changes, making it easier to review individually with whitespace changes hidden.
diff --git a/compiler/rustc_middle/src/query/inner.rs b/compiler/rustc_middle/src/query/inner.rs
@@ -0,0 +1,134 @@
+//! 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::IntoQueryParam;
+use crate::query::erase::{self, Erase, EraseType};
+use crate::ty::TyCtxt;
+
+/// Shared implementation of `tcx.$query(..)` and `tcx.at(span).$query(..)`
+/// for all queries.
+#[inline(always)]
+pub(crate) fn query_get_at<'tcx, Cache>(
+    tcx: TyCtxt<'tcx>,
+    execute_query: fn(TyCtxt<'tcx>, Span, Cache::Key, QueryMode) -> Option<Cache::Value>,
+    query_cache: &Cache,
+    span: Span,
+    key: Cache::Key,
+) -> Cache::Value
+where
+    Cache: QueryCache,
+{
+    let key = key.into_query_param();
+    match try_get_cached(tcx, query_cache, &key) {
+        Some(value) => value,
+        None => execute_query(tcx, span, key, QueryMode::Get).unwrap(),
+    }
+}
+
+/// Shared implementation of `tcx.ensure_ok().$query(..)` for most queries,
+/// and `tcx.ensure_done().$query(..)` for all queries.
+#[inline]
+pub(crate) fn query_ensure<'tcx, Cache>(
+    tcx: TyCtxt<'tcx>,
+    execute_query: fn(TyCtxt<'tcx>, Span, Cache::Key, QueryMode) -> Option<Cache::Value>,
+    query_cache: &Cache,
+    key: Cache::Key,
+    check_cache: bool,
+) where
+    Cache: QueryCache,
+{
+    let key = key.into_query_param();
+    if try_get_cached(tcx, query_cache, &key).is_none() {
+        execute_query(tcx, DUMMY_SP, key, QueryMode::Ensure { check_cache });
+    }
+}
+
+/// Shared implementation of `tcx.ensure_ok().$query(..)` for queries that
+/// have the `return_result_from_ensure_ok` modifier.
+#[inline]
+pub(crate) fn query_ensure_error_guaranteed<'tcx, Cache, T>(
+    tcx: TyCtxt<'tcx>,
+    execute_query: fn(TyCtxt<'tcx>, Span, Cache::Key, QueryMode) -> Option<Cache::Value>,
+    query_cache: &Cache,
+    key: Cache::Key,
+    check_cache: bool,
+) -> Result<(), ErrorGuaranteed>
+where
+    Cache: QueryCache<Value = Erase<Result<T, ErrorGuaranteed>>>,
+    Result<T, ErrorGuaranteed>: EraseType,
+{
+    let key = key.into_query_param();
+    if let Some(res) = try_get_cached(tcx, query_cache, &key) {
+        erase::restore(res).map(drop)
+    } else {
+        execute_query(tcx, DUMMY_SP, key, QueryMode::Ensure { check_cache })
+            .map(erase::restore)
+            .map(|res| res.map(drop))
+            // Either we actually executed the query, which means we got a full `Result`,
+            // or we can just assume the query succeeded, because it was green in the
+            // incremental cache. If it is green, that means that the previous compilation
+            // that wrote to the incremental cache compiles successfully. That is only
+            // possible if the cache entry was `Ok(())`, so we emit that here, without
+            // actually encoding the `Result` in the cache or loading it from there.
+            .unwrap_or(Ok(()))
+    }
+}
+
+/// Common implementation of query feeding, used by `define_feedable!`.
+pub(crate) fn query_feed<'tcx, Cache, Value>(
+    tcx: TyCtxt<'tcx>,
+    dep_kind: DepKind,
+    hasher: Option<fn(&mut StableHashingContext<'_>, &Value) -> Fingerprint>,
+    cache: &Cache,
+    key: Cache::Key,
+    erased: Erase<Value>,
+) where
+    Cache: QueryCache<Value = Erase<Value>>,
+    Cache::Key: DepNodeParams<TyCtxt<'tcx>>,
+    Value: EraseType + Debug,
+{
+    let value = erase::restore::<Value>(erased);
+
+    match try_get_cached(tcx, cache, &key) {
+        Some(old) => {
+            let old = erase::restore::<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))
+                    });
+                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:?}",
+                    ));
+                }
+            } else {
+                // The query is `no_hash`, so we have no way to perform a sanity check.
+                // If feeding the same value multiple times needs to be supported,
+                // 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:?}",
+                )
+            }
+        }
+        None => {
+            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);
+        }
+    }
+}
diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
@@ -70,7 +70,6 @@ use std::sync::Arc;
 use rustc_abi::Align;
 use rustc_arena::TypedArena;
 use rustc_ast::expand::allocator::AllocatorKind;
-use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
 use rustc_data_structures::sorted_map::SortedMap;
 use rustc_data_structures::steal::Steal;
@@ -88,9 +87,7 @@ 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::{
-    QueryCache, QueryMode, QueryStackDeferred, QueryState, try_get_cached,
-};
+use rustc_query_system::query::{QueryMode, QueryStackDeferred, QueryState};
 use rustc_session::Limits;
 use rustc_session::config::{EntryFnType, OptLevel, OutputFilenames, SymbolManglingVersion};
 use rustc_session::cstore::{
@@ -103,6 +100,8 @@ use rustc_span::{DUMMY_SP, Span, Symbol};
 use rustc_target::spec::{PanicStrategy, SanitizerSet};
 use {rustc_abi as abi, rustc_ast as ast, rustc_hir as hir};
 
+pub use self::keys::{AsLocalKey, Key, LocalCrate};
+pub use self::plumbing::{IntoQueryParam, TyCtxtAt, TyCtxtEnsureDone, TyCtxtEnsureOk};
 use crate::infer::canonical::{self, Canonical};
 use crate::lint::LintExpectation;
 use crate::metadata::ModChild;
@@ -119,9 +118,7 @@ use crate::mir::interpret::{
 };
 use crate::mir::mono::{CodegenUnit, CollectionMode, MonoItem, MonoItemPartitions};
 use crate::query::erase::{Erase, erase, restore};
-use crate::query::plumbing::{
-    CyclePlaceholder, DynamicQuery, query_ensure, query_ensure_error_guaranteed, query_get_at,
-};
+use crate::query::plumbing::{CyclePlaceholder, DynamicQuery};
 use crate::traits::query::{
     CanonicalAliasGoal, CanonicalDropckOutlivesGoal, CanonicalImpliedOutlivesBoundsGoal,
     CanonicalMethodAutoderefStepsGoal, CanonicalPredicateGoal, CanonicalTypeOpAscribeUserTypeGoal,
@@ -145,12 +142,11 @@ use crate::{dep_graph, mir, thir};
 
 mod arena_cached;
 pub mod erase;
+pub(crate) mod inner;
 mod keys;
-pub use keys::{AsLocalKey, Key, LocalCrate};
 pub mod on_disk_cache;
 #[macro_use]
 pub mod plumbing;
-pub use plumbing::{IntoQueryParam, TyCtxtAt, TyCtxtEnsureDone, TyCtxtEnsureOk};
 
 // Each of these queries corresponds to a function pointer field in the
 // `Providers` struct for requesting a value of that type, and a method
diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs
@@ -8,7 +8,8 @@ use rustc_query_system::HandleCycleError;
 use rustc_query_system::dep_graph::{DepNodeIndex, SerializedDepNodeIndex};
 pub(crate) use rustc_query_system::query::QueryJobId;
 use rustc_query_system::query::*;
-use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span};
+use rustc_span::{ErrorGuaranteed, Span};
+pub use sealed::IntoQueryParam;
 
 use crate::dep_graph;
 use crate::dep_graph::DepKind;
@@ -165,78 +166,17 @@ impl<'tcx> TyCtxt<'tcx> {
     }
 }
 
-#[inline(always)]
-pub fn query_get_at<'tcx, Cache>(
-    tcx: TyCtxt<'tcx>,
-    execute_query: fn(TyCtxt<'tcx>, Span, Cache::Key, QueryMode) -> Option<Cache::Value>,
-    query_cache: &Cache,
-    span: Span,
-    key: Cache::Key,
-) -> Cache::Value
-where
-    Cache: QueryCache,
-{
-    let key = key.into_query_param();
-    match try_get_cached(tcx, query_cache, &key) {
-        Some(value) => value,
-        None => execute_query(tcx, span, key, QueryMode::Get).unwrap(),
-    }
-}
-
-#[inline]
-pub fn query_ensure<'tcx, Cache>(
-    tcx: TyCtxt<'tcx>,
-    execute_query: fn(TyCtxt<'tcx>, Span, Cache::Key, QueryMode) -> Option<Cache::Value>,
-    query_cache: &Cache,
-    key: Cache::Key,
-    check_cache: bool,
-) where
-    Cache: QueryCache,
-{
-    let key = key.into_query_param();
-    if try_get_cached(tcx, query_cache, &key).is_none() {
-        execute_query(tcx, DUMMY_SP, key, QueryMode::Ensure { check_cache });
-    }
-}
-
-#[inline]
-pub fn query_ensure_error_guaranteed<'tcx, Cache, T>(
-    tcx: TyCtxt<'tcx>,
-    execute_query: fn(TyCtxt<'tcx>, Span, Cache::Key, QueryMode) -> Option<Cache::Value>,
-    query_cache: &Cache,
-    key: Cache::Key,
-    check_cache: bool,
-) -> Result<(), ErrorGuaranteed>
-where
-    Cache: QueryCache<Value = super::erase::Erase<Result<T, ErrorGuaranteed>>>,
-    Result<T, ErrorGuaranteed>: EraseType,
-{
-    let key = key.into_query_param();
-    if let Some(res) = try_get_cached(tcx, query_cache, &key) {
-        super::erase::restore(res).map(drop)
-    } else {
-        execute_query(tcx, DUMMY_SP, key, QueryMode::Ensure { check_cache })
-            .map(super::erase::restore)
-            .map(|res| res.map(drop))
-            // Either we actually executed the query, which means we got a full `Result`,
-            // or we can just assume the query succeeded, because it was green in the
-            // incremental cache. If it is green, that means that the previous compilation
-            // that wrote to the incremental cache compiles successfully. That is only
-            // possible if the cache entry was `Ok(())`, so we emit that here, without
-            // actually encoding the `Result` in the cache or loading it from there.
-            .unwrap_or(Ok(()))
-    }
-}
-
-macro_rules! query_ensure {
+/// Calls either `query_ensure` or `query_ensure_error_guaranteed`, depending
+/// on whether the list of modifiers contains `return_result_from_ensure_ok`.
+macro_rules! query_ensure_select {
     ([]$($args:tt)*) => {
-        query_ensure($($args)*)
+        crate::query::inner::query_ensure($($args)*)
     };
     ([(return_result_from_ensure_ok) $($rest:tt)*]$($args:tt)*) => {
-        query_ensure_error_guaranteed($($args)*).map(|_| ())
+        crate::query::inner::query_ensure_error_guaranteed($($args)*)
     };
     ([$other:tt $($modifiers:tt)*]$($args:tt)*) => {
-        query_ensure!([$($modifiers)*]$($args)*)
+        query_ensure_select!([$($modifiers)*]$($args)*)
     };
 }
 
@@ -432,7 +372,7 @@ macro_rules! define_callbacks {
                 self,
                 key: query_helper_param_ty!($($K)*),
             ) -> ensure_ok_result!([$($modifiers)*]) {
-                query_ensure!(
+                query_ensure_select!(
                     [$($modifiers)*]
                     self.tcx,
                     self.tcx.query_system.fns.engine.$name,
@@ -447,7 +387,7 @@ macro_rules! define_callbacks {
             $($(#[$attr])*
             #[inline(always)]
             pub fn $name(self, key: query_helper_param_ty!($($K)*)) {
-                query_ensure(
+                crate::query::inner::query_ensure(
                     self.tcx,
                     self.tcx.query_system.fns.engine.$name,
                     &self.tcx.query_system.caches.$name,
@@ -472,7 +412,7 @@ macro_rules! define_callbacks {
             #[inline(always)]
             pub fn $name(self, key: query_helper_param_ty!($($K)*)) -> $V
             {
-                restore::<$V>(query_get_at(
+                restore::<$V>(crate::query::inner::query_get_at(
                     self.tcx,
                     self.tcx.query_system.fns.engine.$name,
                     &self.tcx.query_system.caches.$name,
@@ -565,48 +505,19 @@ macro_rules! define_feedable {
 
                 let tcx = self.tcx;
                 let erased = queries::$name::provided_to_erased(tcx, value);
-                let value = restore::<$V>(erased);
                 let cache = &tcx.query_system.caches.$name;
 
+                let dep_kind: dep_graph::DepKind = dep_graph::dep_kinds::$name;
                 let hasher: Option<fn(&mut StableHashingContext<'_>, &_) -> _> = hash_result!([$($modifiers)*]);
-                match try_get_cached(tcx, cache, &key) {
-                    Some(old) => {
-                        let old = restore::<$V>(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))
-                            );
-                            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 {} key={key:?}:\n\
-                                    old value: {old:?}\nnew value: {value:?}",
-                                    stringify!($name),
-                                ));
-                            }
-                        } else {
-                            // The query is `no_hash`, so we have no way to perform a sanity check.
-                            // If feeding the same value multiple times needs to be supported,
-                            // the query should not be marked `no_hash`.
-                            bug!(
-                                "Trying to feed an already recorded value for query {} key={key:?}:\nold value: {old:?}\nnew value: {value:?}",
-                                stringify!($name),
-                            )
-                        }
-                    }
-                    None => {
-                        let dep_node = dep_graph::DepNode::construct(tcx, dep_graph::dep_kinds::$name, &key);
-                        let dep_node_index = tcx.dep_graph.with_feed_task(
-                            dep_node,
-                            tcx,
-                            &value,
-                            hash_result!([$($modifiers)*]),
-                        );
-                        cache.complete(key, erased, dep_node_index);
-                    }
-                }
+
+                $crate::query::inner::query_feed(
+                    tcx,
+                    dep_kind,
+                    hasher,
+                    cache,
+                    key,
+                    erased,
+                );
             }
         })*
     }
@@ -694,10 +605,6 @@ mod sealed {
     }
 }
 
-pub use sealed::IntoQueryParam;
-
-use super::erase::EraseType;
-
 #[derive(Copy, Clone, Debug, HashStable)]
 pub struct CyclePlaceholder(pub ErrorGuaranteed);
 
