Rollup merge of #153120 - Zalathar:execute, r=nnethercote

Clean up some code related to `QueryVTable::execute_query_fn`

This PR is an assortment of small cleanups to code that interacts with `execute_query_fn` in the query vtable.

I also experimented with trying to replace the macro-generated `__rust_end_short_backtrace` functions with a single shared generic function, but I couldn't manage to avoid breaking short backtraces, so I left a note behind to document my attempt.

r? nnethercote (or compiler)

Author: JonathanBrouwer

compiler/rustc_middle/src/query/inner.rs

@@ -35,17 +35,16 @@ where
 #[inline(always)]
 pub(crate) fn query_get_at<'tcx, C>(
     tcx: TyCtxt<'tcx>,
-    execute_query: fn(TyCtxt<'tcx>, Span, C::Key, QueryMode) -> Option<C::Value>,
-    query_cache: &C,
     span: Span,
+    query: &'tcx QueryVTable<'tcx, C>,
     key: C::Key,
 ) -> C::Value
 where
     C: QueryCache,
 {
-    match try_get_cached(tcx, query_cache, &key) {
+    match try_get_cached(tcx, &query.cache, &key) {
         Some(value) => value,
-        None => execute_query(tcx, span, key, QueryMode::Get).unwrap(),
+        None => (query.execute_query_fn)(tcx, span, key, QueryMode::Get).unwrap(),
     }
 }
 
@@ -54,15 +53,14 @@ where
 #[inline]
 pub(crate) fn query_ensure<'tcx, C>(
     tcx: TyCtxt<'tcx>,
-    execute_query: fn(TyCtxt<'tcx>, Span, C::Key, QueryMode) -> Option<C::Value>,
-    query_cache: &C,
+    query: &'tcx QueryVTable<'tcx, C>,
     key: C::Key,
     ensure_mode: EnsureMode,
 ) where
     C: QueryCache,
 {
-    if try_get_cached(tcx, query_cache, &key).is_none() {
-        execute_query(tcx, DUMMY_SP, key, QueryMode::Ensure { ensure_mode });
+    if try_get_cached(tcx, &query.cache, &key).is_none() {
+        (query.execute_query_fn)(tcx, DUMMY_SP, key, QueryMode::Ensure { ensure_mode });
     }
 }
 
@@ -71,8 +69,7 @@ pub(crate) fn query_ensure<'tcx, C>(
 #[inline]
 pub(crate) fn query_ensure_error_guaranteed<'tcx, C, T>(
     tcx: TyCtxt<'tcx>,
-    execute_query: fn(TyCtxt<'tcx>, Span, C::Key, QueryMode) -> Option<C::Value>,
-    query_cache: &C,
+    query: &'tcx QueryVTable<'tcx, C>,
     key: C::Key,
     // This arg is needed to match the signature of `query_ensure`,
     // but should always be `EnsureMode::Ok`.
@@ -84,10 +81,10 @@ where
 {
     assert_matches!(ensure_mode, EnsureMode::Ok);
 
-    if let Some(res) = try_get_cached(tcx, query_cache, &key) {
+    if let Some(res) = try_get_cached(tcx, &query.cache, &key) {
         erase::restore_val(res).map(drop)
     } else {
-        execute_query(tcx, DUMMY_SP, key, QueryMode::Ensure { ensure_mode })
+        (query.execute_query_fn)(tcx, DUMMY_SP, key, QueryMode::Ensure { ensure_mode })
             .map(erase::restore_val)
             .map(|res| res.map(drop))
             // Either we actually executed the query, which means we got a full `Result`,

compiler/rustc_middle/src/query/plumbing.rs

@@ -157,6 +157,15 @@ pub struct QueryVTable<'tcx, C: QueryCache> {
     /// Used when reporting query cycle errors and similar problems.
     pub description_fn: fn(TyCtxt<'tcx>, C::Key) -> String,
 
+    /// Function pointer that is called by the query methods on [`TyCtxt`] and
+    /// friends[^1], after they have checked the in-memory cache and found no
+    /// existing value for this key.
+    ///
+    /// Transitive responsibilities include trying to load a disk-cached value
+    /// if possible (incremental only), invoking the query provider if necessary,
+    /// and putting the obtained value into the in-memory cache.
+    ///
+    /// [^1]: [`TyCtxt`], [`TyCtxtAt`], [`TyCtxtEnsureOk`], [`TyCtxtEnsureDone`]
     pub execute_query_fn: fn(TyCtxt<'tcx>, Span, C::Key, QueryMode) -> Option<C::Value>,
 }
 
@@ -495,8 +504,7 @@ macro_rules! define_callbacks {
                         (crate::query::inner::query_ensure)
                     )(
                         self.tcx,
-                        self.tcx.query_system.query_vtables.$name.execute_query_fn,
-                        &self.tcx.query_system.query_vtables.$name.cache,
+                        &self.tcx.query_system.query_vtables.$name,
                         $crate::query::IntoQueryParam::into_query_param(key),
                         $crate::query::EnsureMode::Ok,
                     )
@@ -511,8 +519,7 @@ macro_rules! define_callbacks {
                 pub fn $name(self, key: query_helper_param_ty!($($K)*)) {
                     crate::query::inner::query_ensure(
                         self.tcx,
-                        self.tcx.query_system.query_vtables.$name.execute_query_fn,
-                        &self.tcx.query_system.query_vtables.$name.cache,
+                        &self.tcx.query_system.query_vtables.$name,
                         $crate::query::IntoQueryParam::into_query_param(key),
                         $crate::query::EnsureMode::Done,
                     );
@@ -540,9 +547,8 @@ macro_rules! define_callbacks {
 
                     erase::restore_val::<$V>(inner::query_get_at(
                         self.tcx,
-                        self.tcx.query_system.query_vtables.$name.execute_query_fn,
-                        &self.tcx.query_system.query_vtables.$name.cache,
                         self.span,
+                        &self.tcx.query_system.query_vtables.$name,
                         $crate::query::IntoQueryParam::into_query_param(key),
                     ))
                 }

compiler/rustc_query_impl/src/execution.rs

@@ -270,6 +270,7 @@ fn wait_for_query<'tcx, C: QueryCache>(
     }
 }
 
+/// Shared main part of both [`execute_query_incr_inner`] and [`execute_query_non_incr_inner`].
 #[inline(never)]
 fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>(
     query: &'tcx QueryVTable<'tcx, C>,
@@ -650,8 +651,10 @@ fn check_if_ensure_can_skip_execution<'tcx, C: QueryCache>(
     }
 }
 
+/// Called by a macro-generated impl of [`QueryVTable::execute_query_fn`],
+/// in non-incremental mode.
 #[inline(always)]
-pub(super) fn get_query_non_incr<'tcx, C: QueryCache>(
+pub(super) fn execute_query_non_incr_inner<'tcx, C: QueryCache>(
     query: &'tcx QueryVTable<'tcx, C>,
     tcx: TyCtxt<'tcx>,
     span: Span,
@@ -662,8 +665,10 @@ pub(super) fn get_query_non_incr<'tcx, C: QueryCache>(
     ensure_sufficient_stack(|| try_execute_query::<C, false>(query, tcx, span, key, None).0)
 }
 
+/// Called by a macro-generated impl of [`QueryVTable::execute_query_fn`],
+/// in incremental mode.
 #[inline(always)]
-pub(super) fn get_query_incr<'tcx, C: QueryCache>(
+pub(super) fn execute_query_incr_inner<'tcx, C: QueryCache>(
     query: &'tcx QueryVTable<'tcx, C>,
     tcx: TyCtxt<'tcx>,
     span: Span,

compiler/rustc_query_impl/src/plumbing.rs

@@ -476,7 +476,12 @@ macro_rules! define_queries {
             use super::super::*;
             use ::rustc_middle::query::erase::{self, Erased};
 
-            pub(crate) mod get_query_incr {
+            // It seems to be important that every query has its own monomorphic
+            // copy of `execute_query_incr` and `execute_query_non_incr`.
+            // Trying to inline these wrapper functions into their generic
+            // "inner" helpers tends to break `tests/run-make/short-ice`.
+
+            pub(crate) mod execute_query_incr {
                 use super::*;
 
                 // Adding `__rust_end_short_backtrace` marker to backtraces so that we emit the frames
@@ -490,7 +495,7 @@ macro_rules! define_queries {
                 ) -> Option<Erased<queries::$name::Value<'tcx>>> {
                     #[cfg(debug_assertions)]
                     let _guard = tracing::span!(tracing::Level::TRACE, stringify!($name), ?key).entered();
-                    execution::get_query_incr(
+                    execution::execute_query_incr_inner(
                         &tcx.query_system.query_vtables.$name,
                         tcx,
                         span,
@@ -500,7 +505,7 @@ macro_rules! define_queries {
                 }
             }
 
-            pub(crate) mod get_query_non_incr {
+            pub(crate) mod execute_query_non_incr {
                 use super::*;
 
                 #[inline(never)]
@@ -510,7 +515,7 @@ macro_rules! define_queries {
                     key: queries::$name::Key<'tcx>,
                     __mode: QueryMode,
                 ) -> Option<Erased<queries::$name::Value<'tcx>>> {
-                    Some(execution::get_query_non_incr(
+                    Some(execution::execute_query_non_incr_inner(
                         &tcx.query_system.query_vtables.$name,
                         tcx,
                         span,
@@ -605,9 +610,9 @@ macro_rules! define_queries {
                     format_value: |value| format!("{:?}", erase::restore_val::<queries::$name::Value<'tcx>>(*value)),
                     description_fn: $crate::queries::_description_fns::$name,
                     execute_query_fn: if incremental {
-                        query_impl::$name::get_query_incr::__rust_end_short_backtrace
+                        query_impl::$name::execute_query_incr::__rust_end_short_backtrace
                     } else {
-                        query_impl::$name::get_query_non_incr::__rust_end_short_backtrace
+                        query_impl::$name::execute_query_non_incr::__rust_end_short_backtrace
                     },
                 }
             }