`QueryEngine` is a struct with one function pointer per query. This
commit removes it by moving each function pointer into the relevant
query's vtable, which is already a collection of function pointers for
that query. This makes things simpler.
Currently type variables that impl `QueryCache` are called either `C` or
`Cache`. I find the former clearer because single char type variables
names are very common and longer type variable names are easy to mistake
for type or trait names -- e.g. I find the trait bound `C: QueryCache`
much easier and faster to read than `Cache: QueryCache`.
`QuerySystem` has these fields:
```
pub states: QueryStates<'tcx>,
pub caches: QueryCaches<'tcx>,
pub query_vtables: PerQueryVTables<'tcx>,
```
Each one has an entry for each query.
Some methods that take a query-specific `QueryVTable` need to access the
corresponding query-specific states and/or caches. So `QueryVTable`
stores the *byte offset* to the relevant fields within `states` and
`caches`, and uses that to (with `unsafe`) access the fields. This is
bizarre, and I hope it made sense in the past when the code was
structured differently.
This commit moves `QueryState` and `QueryCache` inside `QueryVTable`. As
a result, the query-specific states and/or caches are directly
accessible, and no unsafe offset computations are required. Much simpler
and more normal. Also, `QueryCaches` and `QueryStates` are no longer
needed, which makes `define_callbacks!` a bit simpler.
The commit also rename the fields and their getters to `states` and
`caches`.
Clarify how "ensure" queries check whether they can skip execution
The current `check_cache: bool` field in QueryMode is confusing for a few reasons:
- It actually refers to checking the *disk cache*, not the in-memory cache.
- It gives no indication of what condition is being checked, or why.
- It obscures the link between `tcx.ensure_ok()` and `tcx.ensure_done()`, and the actual check.
This PR replaces that field with an `EnsureMode` enum that distinguishes between ensure-ok and ensure-done, and leaves the actual check as an implementation detail of the ensure-done mode.
This PR also renames `ensure_must_run` → `check_if_ensure_can_skip_execution`, and uses a return struct to more clearly indicate how this helper function gives permission for its caller to skip execution of a query that would otherwise be executed. This also inverts the sense of the returned boolean, which was previously “must run” but is now ”skip execution”.
r? nnethercote (or compiler)
It's now `query_vtable` because its return type changed. And thanks to
the previous commit it can be manually inlined in several places. (The
only remaining calls to it are in `make_dep_kind_vtable_for_query`,
which are more challenging to remove.)
`SemiDynamicQueryDispatcher` is just a `QueryVTable` wrapper with an
additional `const FLAGS: QueryFlags` generic parameter that contains
three booleans. This arrangement exists as a performance optimization.
But the performance effects are very small and it adds quite a bit of
complexity to an already overly-complex part of the codebase. If it
didn't exist and somebody proposed adding it and asked me to review, I
almost certainly wouldn't approve it.
This commit removes it. The three booleans in `QueryFlags` are moved
into `QueryVTable` The non-trivial methods of
`SemiDynamicQueryDispatcher` become methods of `QueryVTable`.
Within `define_callbacks!`:
- Use `Key<'tcx>` where possible instead of `$($K)*`.
- Use `Value<'tcx>` where possible instead of `$V`.
In the other `define_*!` macros:
- Use `$K:ty` where possible instead of `$($K:tt)*`.
- Add comments about unused `$K` and `$V` cases.
- Add `()` key types to the special nodes in the `define_dep_nodes!`
invocation for consistency with normal queries.
There is a bunch of plumbing to record the string `"foo"` for each
variant `DepKind::foo`. But that's what the `Debug `impl` now produces.
So this commit removes the plumbing.
It was removed in #115920 to enable it being moved to
`rustc_query_system`, a move that has recently been reversed. It's much
simpler as an enum.
Also:
- Remove the overly complicated `Debug` impl for `DepKind`.
- Remove the trivial `DepKind` associated constants (`NULL` et al.)
- Add an assertion to ensure that the number of `DepKinds` fits within a
`u16`.
- Rename `DEP_KIND_VARIANTS` as `DEP_KIND_NUM_VARIANTS`, to make it
clearer that it's a count, not a collection.
- Use `stringify!` in one place to make the code clearer.
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.
Big query system cleanups
Recent PRs have moved a lot of code from `rustc_query_system` to `rustc_middle` and `rustc_query_impl`, where this code now has access to `TyCtxt`, e.g. rust-lang/rust#152419, rust-lang/rust#152516. As a result, a lot of abstraction and indirection that existed to work around this limitation is no longer necessary. This PR removes a lot of it.
r? @Zalathar
By removing the generic `D` parameter from `DepGraph`, `DepGraphData`,
`CurrentDepGraph`, `SerializedDepGraph`, `SerializedNodeHeader`, and
`EncoderState`.
Most of the files within the `dep_graph` module can be moved wholesale
into `rustc_middle`. But two of them (`mod.rs` and `dep_node.rs`) have
the same name as existing files in `rustc_middle`, so for those I just
copied the contents into the existing files.
The commit also moves `QueryContext` and `incremental_verify_ich*`
because they are tightly intertwined with the dep graph code. And a
couple of error structs moved as well.
This includes the types `QueryInfo`, `QueryJob`, `QueryJobId`,
`QueryWaiter`, `QueryLatch`, and `QueryLatchInfo`.
`CycleError` and `QueryStack*` had to come along too, due to type
interdependencies. The `QueryStack*` types are put into a new submodule
`rustc_middle::query::stack`.
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
Move more query system code
Towards the goal of eliminating `rustc_query_system`, this commit moves some code from `rustc_query_system` to `rustc_middle` and `rustc_query_impl`, and from `rustc_middle` to `rustc_query_impl`.
r? @Zalathar
Clean up query macros for `cache_on_disk_if`
This PR aims to make the macros for dealing with `cache_on_disk_if` a bit easier to read and work with.
There should be no change to compiler behaviour.
The latter is a new module.
As well as the code motion, some other changes were required.
- `QueryJobId` methods became free functions so they could move while
`QueryJobId` itself stayed put. This was so `QueryMap` and
`QueryJobInfo` could be moved.
- Some visibilities in `rustc_query_system` required changing.
- `collect_active_jobs_from_all_queries` is no longer required in `trait
QueryContext`.
This value is a previously-computed hash of the key, so it makes sense to
bundle it with the key inside the guard, since the guard will need it on
completion anyway.
It existed only to bridge the divide between `rustc_query_system` and
`rustc_query_impl`. But enough pieces have been moved from the former to
the latter that the trait is no longer needed. Less indirection and
abstraction makes the code easier to understand.
It takes an `is_anon` argument, but it's now generic over `QueryFlags`,
which contains the same `is_anon` field. So the argument is no longer
necessary.
It's always `SemiDynamicQueryDispatcher`, so we can just use that type
directly. (This requires adding some explicit generic params to
`QueryDispatcherUnerased`.) Less indirection makes the code clearer, and
this is a prerequisite for the next commit, which is a much bigger
simplification.
Query keys must be stably hashable. Currently this requirement is
expressed as a where-clause on `impl QueryDispatcher for
SemiDynamicQueryDispatcher` and a where-clause on
`create_deferred_query_stack_frame`.
This commit removes those where-clause bounds and adds a single bound to
`QueryCache::Key`, which already has some other bounds. I.e. it
consolidates the bounds. It also gives them a name (`QueryCacheKey`) to
avoid repeating them. There is also a related `Key` trait in
`rustc_middle`; it should probably be merged with `QueryCacheKey` in the
future, but not today.
This cleanup helps with the next two commits, which do bigger
rearrangements, and where the where-clauses caused me some difficulties.
The previous commit moved some code from `rustc_query_system`, which
doesn't have access to `TyCtxt` and `QueryCtxt`, to `rustc_query_impl`,
which does. We can now remove quite a bit of indirection.
- Three methods in `trait QueryContext` are no longer needed
(`next_job_id`, `current_query_job`, `start_query`). As a result,
`QueryCtxt`'s trait impls of these methods are changed to inherent
methods.
- `qcx: Q::Qcx` parameters are simplified to `qcx: QueryCtxt<'tcx>`.
- `*qcx.dep_context()` occurrences are simplified to `qcx.tcx`, and
things like `qcx.dep_context().profiler()` become `qcx.tcx.prof`.
- `DepGraphData<<Q::Qcx as HasDepContext>::Deps>` becomes
`DepGraphData<DepsType>`.
In short, various layers of indirection and abstraction are cut away.
The resulting code is simpler, more concrete, and easier to understand.
It's a good demonstration of the benefits of eliminating
`rustc_query_system`, and there will be more to come.
[Note: this commit conceptually moves 75% of a file to another location,
and leaves 25% of it behind. It's impossible to preserve all the git
history. To preserve git history of the moved 75%, in this commit we
rename the file and remove the 25% from it, leaving the code in an
incomplete (uncompilable) state. In the next commit we add back the
25% in the old location.]
We are in the process of eliminating `rustc_query_system`. Chunks of it
are unused by `rustc_middle`, and so can be moved into
`rustc_query_impl`. This commit does some of that.
Mostly it's just moving code from one file to a new file. There are a
couple of non-trivial changes.
- `QueryState` and `ActiveKeyStatus` must remain in `rustc_query_system`
because they are used by `rustc_middle`. But their inherent methods
are not used by `rustc_middle`. So these methods are moved and
converted to free functions.
- The visibility of some things must increase. This includes `DepGraphData`
and some of its methods, which are now used in `rustc_query_impl`.
This is a bit annoying but seems hard to avoid.
What little is left behind in
`compiler/rustc_query_system/src/query/plumbing.rs` will be able to
moved into `rustc_query_impl` or `rustc_middle` in the future.
Incorporate query description functions into `QueryVTable`
Putting a `desc` function in each query vtable reduces the amount of parameter juggling required when creating query stack frames, because almost all of the necessary information can be found in the vtable.
There should be no change to compiler output.
Nicholas Nethercote