Clean up some code related to `QueryVTable::execute_query_fn`
This PR is an assortment of small cleanups to code that interacts with `execute_query_fn` in the query vtable.
I also experimented with trying to replace the macro-generated `__rust_end_short_backtrace` functions with a single shared generic function, but I couldn't manage to avoid breaking short backtraces, so I left a note behind to document my attempt.
r? nnethercote (or compiler)
It's currently in `dep_node.rs`, along with a blanket impl. Specific
impls are in `dep_node_key.rs`. This commit moves the trait and the
blanket impl into `dep_node_key.rs`, so everything is in one place.
This function is tricky to document, and there's more that could be said here,
but I don't want to take up too much vertical space, or add too much risk of
details becoming inaccurate over time.
All three of these functions were previously taking `cache` and
`execute_query_fn` as separate arguments, but after some other recent
simplifications we can just pass `&'tcx QueryVTable<'tcx, C>` instead.
This makes it easier to see where `execute_query_fn` is actually called.
`QuerySystem` has two function pointers: `encode_query_results` and
`try_mark_green`. These exist so that `rustc_middle` can call functions
from upstream crates.
But we have a more general mechanism for that: hooks. So this commit
converts these two cases into hooks.
`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)
Remove `rustc_feedable_queries` and `define_feedable` macros.
The can be folded into `rustc_with_all_queries` and `define_callbacks`. Details in individual commits.
r? @oli-obk
`rustc_queries!` produces two macros: `rustc_with_all_queries` and
`rustc_feedable_queries`. The latter is similar to the former but only
includes feedable queries.
But feedable queries don't need a separate mechanism because we can
identify feedable queries within `define_callbacks!` by just looking for
the `feedable` modifier. (That's what we do with every modifier other
than `feedable`.)
This commit removes the special handling and treats feedable queries
like everything else.
Note that this commit exposes and fixes a latent doc bug. The doc
comment for query `explicit_predicates_of` links to
`Self::predicates_of`. `explicit_predicates_of` is a feedable query but
`predicates_of` is not, so for `TyCtxtFeed` this link is invalid. This
wasn't manifesting because `TyCtxtFeed` wasn't getting doc comments
attached. It now is, so I changed the link to `TyCtxt::predicates_of`.
Superficial tweaks to the query modifier docs in `rustc_middle::query::modifiers`
This PR sorts the dummy items in the `modifiers` module, makes them non-pub to speed up find-all-references, and adds some extra explanation of what the dummy modifier items are for.
(These dummy items mostly just exist to carry documentation, and have no actual effect on compiler behaviour.)
fix interpreter tracing output
https://github.com/rust-lang/rust/pull/144708 accidentally changed the output of `MIRI_LOG=info <miri run>` in two ways:
- by adding `stmt=`/`terminator=` prefixes
- by changing the statement printing to be a verbose debug version instead of MIR pretty-printing
This fixes both of these:
- use explicit format strings to avoid the prefixes
- fix inconsistency in Debug impls for MIR types: now both TerminatorKind and StatementKind are pretty-printed, and Terminator and Statement just forward to the *Kind output
GVN: consider constants of primitive types as deterministic
*[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust/pull/149366)*
GVN separates MIR constants into deterministic and non-deterministic constants. Deterministic constants are defined here as: gives the exact same value each time it is evaluated.
This was mainly useful because of `ConstValue::Slice` that generated an extra `AllocId` each time it appeared in the MIR. That variant has been removed. This is still useful for valtrees that hold references, which generate a fresh `AllocId` for each evaluation.
This PR proposes to consider all constants of primitive type to be deterministic. If a constant of primitive type passes validation, then it does not contain provenance, so we have no risk of having a reference becoming different `AllocId`s. In particular, valtrees only are leaves.
r? @ghost for perf
Use `HashStable` derive in more places
This applies `HashStable` derive in a couple more places. Also `stable_hasher` is declared for `HashStable_NoContext`.
`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`.
Josh Stone