Remove QueryCtxt and trait HasDepContext#152704
Remove QueryCtxt and trait HasDepContext#152704Zalathar wants to merge 3 commits intorust-lang:mainfrom
QueryCtxt and trait HasDepContext#152704Conversation
rustbot
added
A-query-system
| self.assert_dep_node_not_yet_allocated_in_current_session(tcx.sess, &dep_node, || { | ||
| format!( | ||
| "forcing query with already existing `DepNode`\n\ | ||
| - query-key: {task_arg:?}\n\ |
There was a problem hiding this comment.
This won't just print the query key now.
There was a problem hiding this comment.
It will print a tuple of (query_name, key), which seems fine.
| // Delegate to another function to actually execute the query job. | ||
| let (result, dep_node_index) = if INCR { | ||
| execute_job_incr(query, qcx, qcx.tcx.dep_graph.data().unwrap(), key, dep_node, id) | ||
| execute_job_incr(query, tcx, tcx.dep_graph.data().unwrap(), key, dep_node, id) |
There was a problem hiding this comment.
We could now just pass in tcx and get the dep graph data later (in try_load_from_disk_and_cache_in_memory).
There was a problem hiding this comment.
That duplicates the unwrap(), so DepGraphData is passed around to indicate incremental is enabled.
There was a problem hiding this comment.
I have pushed the call to tcx.dep_graph.data().unwrap() down into execute_job_incr, and incorporated that change into the commit that removes QueryCtxt, because it touches the affected lines anyway.
Pushing the call down any further is not obviously better to me, so I stopped there for now.
|
r=me with the nits addressed, and merging should wait for #152703. |
nnethercote
added
S-waiting-on-author
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
This struct was only wrapping `TyCtxt` in order to implement traits that were removed by RUST-152636. This commit also slightly simplifies the signature of `execute_job_incr`, by having it call `tcx.dep_graph.data()` internally.
The need for a `HasDepContext` impl on tuples can be avoided by passing the query vtable as part of an argument tuple instead.
@rustbot blocked |
rustbot
added
S-blocked
5 participants
rustc_query_system#152703 to reduce conflicts.With the
QueryContexttrait removed, wrapper structQueryCtxtno longer serves a purpose and can be replaced withTyCtxteverywhere.After that, the only obstacle to removing trait
HasDepContextisDepGraph::with_task, which uses the trait to allow passing both aTyCtxtand a query vtable through the context argument. But we can achieve the same result by passing the vtable through the other argument instead, in a tuple alongside the query key.r? nnethercote