Unrolled build for #152434

Rollup merge of #152434 - Zalathar:call-query, r=nnethercote

Clarify names of `QueryVTable` functions for "executing" a query

In the query system, there are several layers of functions involved in “executing” a query, with very different responsibilities at each layer, making it important to be able to tell them apart.

This PR renames two of the function pointers in `QueryVTable`, along with their associated helper functions, to hopefully do a better job of indicating what their actual responsibilities are.

r? nnethercote

Author: rust-timer

compiler/rustc_middle/src/query/plumbing.rs

@@ -49,8 +49,21 @@ pub struct QueryVTable<'tcx, C: QueryCache> {
     // Offset of this query's cache field in the QueryCaches struct
     pub query_cache: usize,
     pub will_cache_on_disk_for_key_fn: Option<WillCacheOnDiskForKeyFn<'tcx, C::Key>>,
-    pub execute_query: fn(tcx: TyCtxt<'tcx>, k: C::Key) -> C::Value,
-    pub compute_fn: fn(tcx: TyCtxt<'tcx>, key: C::Key) -> C::Value,
+
+    /// 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.
+    ///
+    /// 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<TryLoadFromDiskFn<'tcx, C::Key, C::Value>>,
     pub is_loadable_from_disk_fn: Option<IsLoadableFromDiskFn<'tcx, C::Key>>,
     pub hash_result: HashResult<C::Value>,

compiler/rustc_query_impl/src/execution.rs

@@ -416,7 +416,8 @@ fn execute_job_non_incr<'tcx, C: QueryCache, const FLAGS: QueryFlags>(
     }
 
     let prof_timer = qcx.tcx.prof.query_provider();
-    let result = qcx.start_query(job_id, query.depth_limit(), || query.compute(qcx, key));
+    // Call the query provider.
+    let result = qcx.start_query(job_id, query.depth_limit(), || query.invoke_provider(qcx, key));
     let dep_node_index = qcx.tcx.dep_graph.next_virtual_depnode_index();
     prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 
@@ -459,18 +460,21 @@ fn execute_job_incr<'tcx, C: QueryCache, const FLAGS: QueryFlags>(
 
     let (result, dep_node_index) = qcx.start_query(job_id, query.depth_limit(), || {
         if query.anon() {
-            return dep_graph_data
-                .with_anon_task_inner(qcx.tcx, query.dep_kind(), || query.compute(qcx, key));
+            // Call the query provider inside an anon task.
+            return dep_graph_data.with_anon_task_inner(qcx.tcx, query.dep_kind(), || {
+                query.invoke_provider(qcx, key)
+            });
         }
 
         // `to_dep_node` is expensive for some `DepKind`s.
         let dep_node = dep_node_opt.unwrap_or_else(|| query.construct_dep_node(qcx.tcx, &key));
 
+        // Call the query provider.
         dep_graph_data.with_task(
             dep_node,
             (qcx, query),
             key,
-            |(qcx, query), key| query.compute(qcx, key),
+            |(qcx, query), key| query.invoke_provider(qcx, key),
             query.hash_result(),
         )
     });
@@ -547,7 +551,8 @@ fn try_load_from_disk_and_cache_in_memory<'tcx, C: QueryCache, const FLAGS: Quer
     let prof_timer = qcx.tcx.prof.query_provider();
 
     // The dep-graph for this computation is already in-place.
-    let result = qcx.tcx.dep_graph.with_ignore(|| query.compute(qcx, *key));
+    // Call the query provider.
+    let result = qcx.tcx.dep_graph.with_ignore(|| query.invoke_provider(qcx, *key));
 
     prof_timer.finish_with_query_invocation_id(dep_node_index.into());
 

compiler/rustc_query_impl/src/lib.rs

@@ -110,15 +110,18 @@ impl<'tcx, C: QueryCache, const FLAGS: QueryFlags> SemiDynamicQueryDispatcher<'t
         }
     }
 
-    // Don't use this method to compute query results, instead use the methods on TyCtxt.
+    /// Calls `tcx.$query(key)` for this query, and discards the returned value.
+    /// See [`QueryVTable::call_query_method_fn`] for details of this strange operation.
     #[inline(always)]
-    fn execute_query(self, tcx: TyCtxt<'tcx>, key: C::Key) -> C::Value {
-        (self.vtable.execute_query)(tcx, key)
+    fn call_query_method(self, tcx: TyCtxt<'tcx>, key: C::Key) {
+        (self.vtable.call_query_method_fn)(tcx, key)
     }
 
+    /// Calls the actual provider function for this query.
+    /// See [`QueryVTable::invoke_provider_fn`] for more details.
     #[inline(always)]
-    fn compute(self, qcx: QueryCtxt<'tcx>, key: C::Key) -> C::Value {
-        (self.vtable.compute_fn)(qcx.tcx, key)
+    fn invoke_provider(self, qcx: QueryCtxt<'tcx>, key: C::Key) -> C::Value {
+        (self.vtable.invoke_provider_fn)(qcx.tcx, key)
     }
 
     #[inline(always)]

compiler/rustc_query_impl/src/plumbing.rs

@@ -458,7 +458,9 @@ fn try_load_from_on_disk_cache<'tcx, C: QueryCache, const FLAGS: QueryFlags>(
         panic!("Failed to recover key for {:?} with hash {}", dep_node, dep_node.hash)
     });
     if query.will_cache_on_disk_for_key(tcx, &key) {
-        let _ = query.execute_query(tcx, key);
+        // Call `tcx.$query(key)` for its side-effect of loading the disk-cached
+        // value into memory.
+        query.call_query_method(tcx, key);
     }
 }
 
@@ -626,14 +628,15 @@ macro_rules! define_queries {
                 }
             }
 
-            /// Defines a `compute` function for this query, to be used as a
-            /// function pointer in the query's vtable.
-            mod compute_fn {
+            /// Defines an `invoke_provider` function that calls the query's provider,
+            /// to be used as a function pointer in the query's vtable.
+            ///
+            /// To mark a short-backtrace boundary, the function's actual name
+            /// (after demangling) must be `__rust_begin_short_backtrace`.
+            mod invoke_provider_fn {
                 use super::*;
                 use ::rustc_middle::queries::$name::{Key, Value, provided_to_erased};
 
-                /// This function would be named `compute`, but we also want it
-                /// to mark the boundaries of an omitted region in backtraces.
                 #[inline(never)]
                 pub(crate) fn __rust_begin_short_backtrace<'tcx>(
                     tcx: TyCtxt<'tcx>,
@@ -644,10 +647,13 @@ macro_rules! define_queries {
 
                     // Call the actual provider function for this query.
                     let provided_value = call_provider!([$($modifiers)*][tcx, $name, key]);
+
                     rustc_middle::ty::print::with_reduced_queries!({
                         tracing::trace!(?provided_value);
                     });
 
+                    // Erase the returned value, because `QueryVTable` uses erased values.
+                    // For queries with `arena_cache`, this also arena-allocates the value.
                     provided_to_erased(tcx, provided_value)
                 }
             }
@@ -667,8 +673,12 @@ macro_rules! define_queries {
                     } {
                         None
                     }),
-                    execute_query: |tcx, key| erase::erase_val(tcx.$name(key)),
-                    compute_fn: self::compute_fn::__rust_begin_short_backtrace,
+                    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,
                     try_load_from_disk_fn: if_cache_on_disk!([$($modifiers)*] {
                         Some(|tcx, key, prev_index, index| {
                             // Check the `cache_on_disk_if` condition for this key.