Skip to content

Commit db58395

Browse files
committed
Don't use HasDepContext in DepGraph::with_task
The need for a `HasDepContext` impl on tuples can be avoided by passing the query vtable as part of an argument tuple instead.
1 parent fbb34d8 commit db58395

4 files changed

Lines changed: 46 additions & 53 deletions

File tree

compiler/rustc_middle/src/dep_graph/graph.rs

Lines changed: 28 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -268,17 +268,17 @@ impl DepGraph {
268268
}
269269

270270
#[inline(always)]
271-
pub fn with_task<'tcx, Ctxt: HasDepContext<'tcx>, A: Debug, R>(
271+
pub fn with_task<'tcx, A: Debug, R>(
272272
&self,
273-
key: DepNode,
274-
cx: Ctxt,
275-
arg: A,
276-
task: fn(Ctxt, A) -> R,
273+
dep_node: DepNode,
274+
tcx: TyCtxt<'tcx>,
275+
task_arg: A,
276+
task_fn: fn(tcx: TyCtxt<'tcx>, task_arg: A) -> R,
277277
hash_result: Option<fn(&mut StableHashingContext<'_>, &R) -> Fingerprint>,
278278
) -> (R, DepNodeIndex) {
279279
match self.data() {
280-
Some(data) => data.with_task(key, cx, arg, task, hash_result),
281-
None => (task(cx, arg), self.next_virtual_depnode_index()),
280+
Some(data) => data.with_task(dep_node, tcx, task_arg, task_fn, hash_result),
281+
None => (task_fn(tcx, task_arg), self.next_virtual_depnode_index()),
282282
}
283283
}
284284

@@ -310,66 +310,50 @@ impl DepGraphData {
310310
/// prevent implicit 'leaks' of tracked state into the task (which
311311
/// could then be read without generating correct edges in the
312312
/// dep-graph -- see the [rustc dev guide] for more details on
313-
/// the dep-graph). To this end, the task function gets exactly two
314-
/// pieces of state: the context `cx` and an argument `arg`. Both
315-
/// of these bits of state must be of some type that implements
316-
/// `DepGraphSafe` and hence does not leak.
317-
///
318-
/// The choice of two arguments is not fundamental. One argument
319-
/// would work just as well, since multiple values can be
320-
/// collected using tuples. However, using two arguments works out
321-
/// to be quite convenient, since it is common to need a context
322-
/// (`cx`) and some argument (e.g., a `DefId` identifying what
323-
/// item to process).
313+
/// the dep-graph).
324314
///
325-
/// For cases where you need some other number of arguments:
326-
///
327-
/// - If you only need one argument, just use `()` for the `arg`
328-
/// parameter.
329-
/// - If you need 3+ arguments, use a tuple for the
330-
/// `arg` parameter.
315+
/// Therefore, the task function takes a `TyCtxt`, plus exactly one
316+
/// additional argument, `task_arg`. The additional argument type can be
317+
/// `()` if no argument is needed, or a tuple if multiple arguments are
318+
/// needed.
331319
///
332320
/// [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/queries/incremental-compilation.html
333321
#[inline(always)]
334-
pub fn with_task<'tcx, Ctxt: HasDepContext<'tcx>, A: Debug, R>(
322+
pub fn with_task<'tcx, A: Debug, R>(
335323
&self,
336-
key: DepNode,
337-
cx: Ctxt,
338-
arg: A,
339-
task: fn(Ctxt, A) -> R,
324+
dep_node: DepNode,
325+
tcx: TyCtxt<'tcx>,
326+
task_arg: A,
327+
task_fn: fn(tcx: TyCtxt<'tcx>, task_arg: A) -> R,
340328
hash_result: Option<fn(&mut StableHashingContext<'_>, &R) -> Fingerprint>,
341329
) -> (R, DepNodeIndex) {
342330
// If the following assertion triggers, it can have two reasons:
343331
// 1. Something is wrong with DepNode creation, either here or
344332
// in `DepGraph::try_mark_green()`.
345333
// 2. Two distinct query keys get mapped to the same `DepNode`
346334
// (see for example #48923).
347-
self.assert_dep_node_not_yet_allocated_in_current_session(
348-
cx.dep_context().sess,
349-
&key,
350-
|| {
351-
format!(
352-
"forcing query with already existing `DepNode`\n\
353-
- query-key: {arg:?}\n\
354-
- dep-node: {key:?}"
355-
)
356-
},
357-
);
335+
self.assert_dep_node_not_yet_allocated_in_current_session(tcx.sess, &dep_node, || {
336+
format!(
337+
"forcing query with already existing `DepNode`\n\
338+
- query-key: {task_arg:?}\n\
339+
- dep-node: {dep_node:?}"
340+
)
341+
});
358342

359-
let with_deps = |task_deps| with_deps(task_deps, || task(cx, arg));
360-
let (result, edges) = if cx.dep_context().is_eval_always(key.kind) {
343+
let with_deps = |task_deps| with_deps(task_deps, || task_fn(tcx, task_arg));
344+
let (result, edges) = if tcx.is_eval_always(dep_node.kind) {
361345
(with_deps(TaskDepsRef::EvalAlways), EdgesVec::new())
362346
} else {
363347
let task_deps = Lock::new(TaskDeps::new(
364348
#[cfg(debug_assertions)]
365-
Some(key),
349+
Some(dep_node),
366350
0,
367351
));
368352
(with_deps(TaskDepsRef::Allow(&task_deps)), task_deps.into_inner().reads)
369353
};
370354

371355
let dep_node_index =
372-
self.hash_result_and_alloc_node(cx.dep_context(), key, edges, &result, hash_result);
356+
self.hash_result_and_alloc_node(tcx, dep_node, edges, &result, hash_result);
373357

374358
(result, dep_node_index)
375359
}

compiler/rustc_middle/src/dep_graph/mod.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,6 @@ impl<'tcx> HasDepContext<'tcx> for TyCtxt<'tcx> {
3535
}
3636
}
3737

38-
impl<'tcx, T: HasDepContext<'tcx>, Q: Copy> HasDepContext<'tcx> for (T, Q) {
39-
fn dep_context(&self) -> TyCtxt<'tcx> {
40-
self.0.dep_context()
41-
}
42-
}
43-
4438
/// Describes the contents of the fingerprint generated by a given query.
4539
///
4640
/// This is mainly for determining whether and how we can reconstruct a key

compiler/rustc_query_impl/src/execution.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -475,9 +475,9 @@ fn execute_job_incr<'tcx, C: QueryCache, const FLAGS: QueryFlags>(
475475
// Call the query provider.
476476
dep_graph_data.with_task(
477477
dep_node,
478-
(tcx, query),
479-
key,
480-
|(tcx, query), key| query.invoke_provider(tcx, key),
478+
tcx,
479+
(query, key),
480+
|tcx, (query, key)| query.invoke_provider(tcx, key),
481481
query.hash_result(),
482482
)
483483
});

compiler/rustc_query_impl/src/lib.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#![feature(try_blocks)]
1010
// tidy-alphabetical-end
1111

12+
use std::fmt;
1213
use std::marker::ConstParamTy;
1314

1415
use rustc_data_structures::sync::AtomicU64;
@@ -76,6 +77,20 @@ impl<'tcx, C: QueryCache, const FLAGS: QueryFlags> Clone
7677
}
7778
}
7879

80+
impl<'tcx, C: QueryCache, const FLAGS: QueryFlags> fmt::Debug
81+
for SemiDynamicQueryDispatcher<'tcx, C, FLAGS>
82+
{
83+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
84+
// When debug-printing a query dispatcher (e.g. for ICE or tracing),
85+
// just print the query name to know what query we're dealing with.
86+
// The other fields and flags are probably just unhelpful noise.
87+
//
88+
// If there is need for a more detailed dump of all flags and fields,
89+
// consider writing a separate dump method and calling it explicitly.
90+
f.write_str(self.name())
91+
}
92+
}
93+
7994
impl<'tcx, C: QueryCache, const FLAGS: QueryFlags> SemiDynamicQueryDispatcher<'tcx, C, FLAGS> {
8095
#[inline(always)]
8196
fn name(self) -> &'static str {

0 commit comments

Comments
 (0)