Skip to content

Commit 0de45db

Browse files
committed
Clarify names of QueryVTable functions for "executing" a query
This also changes the signature of `call_query_method` to not return a value, because its only caller immediately discards the value anyway.
1 parent 7b25457 commit 0de45db

4 files changed

Lines changed: 51 additions & 20 deletions

File tree

compiler/rustc_middle/src/query/plumbing.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,21 @@ pub struct QueryVTable<'tcx, C: QueryCache> {
4949
// Offset of this query's cache field in the QueryCaches struct
5050
pub query_cache: usize,
5151
pub will_cache_on_disk_for_key_fn: Option<WillCacheOnDiskForKeyFn<'tcx, C::Key>>,
52-
pub execute_query: fn(tcx: TyCtxt<'tcx>, k: C::Key) -> C::Value,
53-
pub compute_fn: fn(tcx: TyCtxt<'tcx>, key: C::Key) -> C::Value,
52+
53+
/// Function pointer that calls `tcx.$query(key)` for this query and
54+
/// discards the returned value.
55+
///
56+
/// This is a weird thing to be doing, and probably not what you want.
57+
/// It is used for loading query results from disk-cache in some cases.
58+
pub call_query_method_fn: fn(tcx: TyCtxt<'tcx>, key: C::Key),
59+
60+
/// Function pointer that actually calls this query's provider.
61+
/// Also performs some associated secondary tasks; see the macro-defined
62+
/// implementation in `mod invoke_provider_fn` for more details.
63+
///
64+
/// This should be the only code that calls the provider function.
65+
pub invoke_provider_fn: fn(tcx: TyCtxt<'tcx>, key: C::Key) -> C::Value,
66+
5467
pub try_load_from_disk_fn: Option<TryLoadFromDiskFn<'tcx, C::Key, C::Value>>,
5568
pub is_loadable_from_disk_fn: Option<IsLoadableFromDiskFn<'tcx, C::Key>>,
5669
pub hash_result: HashResult<C::Value>,

compiler/rustc_query_impl/src/execution.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,8 @@ fn execute_job_non_incr<'tcx, C: QueryCache, const FLAGS: QueryFlags>(
416416
}
417417

418418
let prof_timer = qcx.tcx.prof.query_provider();
419-
let result = qcx.start_query(job_id, query.depth_limit(), || query.compute(qcx, key));
419+
// Call the query provider.
420+
let result = qcx.start_query(job_id, query.depth_limit(), || query.invoke_provider(qcx, key));
420421
let dep_node_index = qcx.tcx.dep_graph.next_virtual_depnode_index();
421422
prof_timer.finish_with_query_invocation_id(dep_node_index.into());
422423

@@ -459,18 +460,21 @@ fn execute_job_incr<'tcx, C: QueryCache, const FLAGS: QueryFlags>(
459460

460461
let (result, dep_node_index) = qcx.start_query(job_id, query.depth_limit(), || {
461462
if query.anon() {
462-
return dep_graph_data
463-
.with_anon_task_inner(qcx.tcx, query.dep_kind(), || query.compute(qcx, key));
463+
// Call the query provider inside an anon task.
464+
return dep_graph_data.with_anon_task_inner(qcx.tcx, query.dep_kind(), || {
465+
query.invoke_provider(qcx, key)
466+
});
464467
}
465468

466469
// `to_dep_node` is expensive for some `DepKind`s.
467470
let dep_node = dep_node_opt.unwrap_or_else(|| query.construct_dep_node(qcx.tcx, &key));
468471

472+
// Call the query provider.
469473
dep_graph_data.with_task(
470474
dep_node,
471475
(qcx, query),
472476
key,
473-
|(qcx, query), key| query.compute(qcx, key),
477+
|(qcx, query), key| query.invoke_provider(qcx, key),
474478
query.hash_result(),
475479
)
476480
});
@@ -547,7 +551,8 @@ fn try_load_from_disk_and_cache_in_memory<'tcx, C: QueryCache, const FLAGS: Quer
547551
let prof_timer = qcx.tcx.prof.query_provider();
548552

549553
// The dep-graph for this computation is already in-place.
550-
let result = qcx.tcx.dep_graph.with_ignore(|| query.compute(qcx, *key));
554+
// Call the query provider.
555+
let result = qcx.tcx.dep_graph.with_ignore(|| query.invoke_provider(qcx, *key));
551556

552557
prof_timer.finish_with_query_invocation_id(dep_node_index.into());
553558

compiler/rustc_query_impl/src/lib.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,18 @@ impl<'tcx, C: QueryCache, const FLAGS: QueryFlags> SemiDynamicQueryDispatcher<'t
107107
}
108108
}
109109

110-
// Don't use this method to compute query results, instead use the methods on TyCtxt.
110+
/// Calls `tcx.$query(key)` for this query, and discards the returned value.
111+
/// See [`QueryVTable::call_query_method_fn`] for details of this strange operation.
111112
#[inline(always)]
112-
fn execute_query(self, tcx: TyCtxt<'tcx>, key: C::Key) -> C::Value {
113-
(self.vtable.execute_query)(tcx, key)
113+
fn call_query_method(self, tcx: TyCtxt<'tcx>, key: C::Key) {
114+
(self.vtable.call_query_method_fn)(tcx, key)
114115
}
115116

117+
/// Calls the actual provider function for this query.
118+
/// See [`QueryVTable::invoke_provider_fn`] for more details.
116119
#[inline(always)]
117-
fn compute(self, qcx: QueryCtxt<'tcx>, key: C::Key) -> C::Value {
118-
(self.vtable.compute_fn)(qcx.tcx, key)
120+
fn invoke_provider(self, qcx: QueryCtxt<'tcx>, key: C::Key) -> C::Value {
121+
(self.vtable.invoke_provider_fn)(qcx.tcx, key)
119122
}
120123

121124
#[inline(always)]

compiler/rustc_query_impl/src/plumbing.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,9 @@ fn try_load_from_on_disk_cache<'tcx, C: QueryCache, const FLAGS: QueryFlags>(
457457
panic!("Failed to recover key for {:?} with hash {}", dep_node, dep_node.hash)
458458
});
459459
if query.will_cache_on_disk_for_key(tcx, &key) {
460-
let _ = query.execute_query(tcx, key);
460+
// Call `tcx.$query(key)` for its side-effect of loading the disk-cached
461+
// value into memory.
462+
query.call_query_method(tcx, key);
461463
}
462464
}
463465

@@ -625,14 +627,15 @@ macro_rules! define_queries {
625627
}
626628
}
627629

628-
/// Defines a `compute` function for this query, to be used as a
629-
/// function pointer in the query's vtable.
630-
mod compute_fn {
630+
/// Defines an `invoke_provider` function that calls the query's provider,
631+
/// to be used as a function pointer in the query's vtable.
632+
///
633+
/// To mark a short-backtrace boundary, the function's actual name
634+
/// (after demangling) must be `__rust_begin_short_backtrace`.
635+
mod invoke_provider_fn {
631636
use super::*;
632637
use ::rustc_middle::queries::$name::{Key, Value, provided_to_erased};
633638

634-
/// This function would be named `compute`, but we also want it
635-
/// to mark the boundaries of an omitted region in backtraces.
636639
#[inline(never)]
637640
pub(crate) fn __rust_begin_short_backtrace<'tcx>(
638641
tcx: TyCtxt<'tcx>,
@@ -643,10 +646,13 @@ macro_rules! define_queries {
643646

644647
// Call the actual provider function for this query.
645648
let provided_value = call_provider!([$($modifiers)*][tcx, $name, key]);
649+
646650
rustc_middle::ty::print::with_reduced_queries!({
647651
tracing::trace!(?provided_value);
648652
});
649653

654+
// Erase the returned value, because `QueryVTable` uses erased values.
655+
// For queries with `arena_cache`, this also arena-allocates the value.
650656
provided_to_erased(tcx, provided_value)
651657
}
652658
}
@@ -666,8 +672,12 @@ macro_rules! define_queries {
666672
} {
667673
None
668674
}),
669-
execute_query: |tcx, key| erase::erase_val(tcx.$name(key)),
670-
compute_fn: self::compute_fn::__rust_begin_short_backtrace,
675+
call_query_method_fn: |tcx, key| {
676+
// Call the query method for its side-effect of loading a value
677+
// from disk-cache; the caller doesn't need the value.
678+
let _ = tcx.$name(key);
679+
},
680+
invoke_provider_fn: self::invoke_provider_fn::__rust_begin_short_backtrace,
671681
try_load_from_disk_fn: if_cache_on_disk!([$($modifiers)*] {
672682
Some(|tcx, key, prev_index, index| {
673683
// Check the `cache_on_disk_if` condition for this key.

0 commit comments

Comments
 (0)