There are three query vtable functions related to the `cache_on_disk_if`
modifier:
- `will_cache_on_disk_for_key_fn`
- `try_load_from_disk_fn`
- `is_loadable_from_disk_fn`
These are all function ptrs within an `Option`. They each have a wrapper
that returns `false`/`None` if the function ptr is missing.
This commit removes the `Option` wrappers. In the `None` case we now set
the function ptr to a trivial closure that returns `false`/`None`. The
commit also removes some typedefs that each have a single use. All this
is a bit simpler, with less indirection.
Note that `QueryVTable::hash_value_fn` is still an `Option`. Unlike the
above three functions, which have trivial behaviour in the `None` case,
`hash_value_fn` is used in ways where the `Some`/`None` distinction is
more meaningful.
This removes the `cycle_fatal` query modifier as it has no effect on its current users.
The default `CycleErrorHandling::Error` mode does the same as `cycle_fatal` when the default impl of `FromCycleError` is used. The return types of queries using `cycle_fatal` however have no specialized `FromCycleError` impl.
Rejig `rustc_with_all_queries!`
There are three things relating to `rustc_with_all_queries!` that have been bugging me.
- `rustc_with_all_queries!`'s ability to receive `extra_fake_queries` lines like `[] fn Null(()) -> (),` where the only real thing is the `Null`, and everything is just pretending to be a normal query, ugh.
- `make_dep_kind_array!`: a macro produced by one macro (`define_dep_nodes!`) and used by another macro (`define_queries!`) in another crate, ugh.
- The `_dep_kind_vtable_ctors` module, which is a special module with no actual code that serves just a way of collecting vtable constructors from two different places so they can be referred to by `make_dep_kind_array!`, ugh.
By making some adjustments to how `rustc_with_all_queries!` works, all three of these things are eliminated.
r? @Zalathar
`rustc_with_all_queries` currently provides information about all
queries. Also, a caller can provide an ad hoc list of extra non-queries.
This is used by `define_queries` for non-query dep kinds: `Null`, `Red`,
etc. This is pretty hacky.
This commit changes `rustc_with_all_queries` so that the non-queries
information is available to all callers. (Some callers ignore the
non-query information.) This is done by adding `non_query` entries to
the primary list of queries in `rustc_queries!`.
This commit:
- Renames the query vtable field `hash_result` to `hash_value_fn`
- Removes the unhelpful `HashResult` type alias, which was hiding an
important layer of `Option`
- Replaces the cryptic `hash_result!` helper macro with a more straightforward
`if_no_hash!` helper, in line with other modifier-checking macros
- Renames a few identifiers to refer to a query's return value as `value`
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)
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.)
`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`.
`IntoQueryParam` is a trait that lets query callers be a bit sloppy with
the passed-in key.
- Types similar to `DefId` will be auto-converted to `DefId`. Likewise
for `LocalDefId`.
- Reference types will be auto-derefed.
The auto-conversion is genuinely useful; the auto-derefing much less so.
In practice it's only used for passing `&DefId` to queries that accept
`DefId`, which is an anti-pattern because `DefId` is marked with
`#[rustc_pass_by_value]`.
This commit removes the auto-deref impl and makes the necessary sigil
adjustments. (I generally avoid using `*` to deref manually at call
sites, preferring to deref via `&` in patterns or via `*` in match
expressions. Mostly because that way a single deref often covers
multiple call sites.)
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.
It doesn't make much sense to put the query doc comments on these
structs. (The doc comments *are* put on various query functions, where
it makes more sense.)
So that all `$( ... $name ... )*` repetitions are spread across multiple
lines, even if they all fit on one line. I find this much easier to read
because the `$name` repetition is the main feature of these macros.
There are six small macros used in `define_callbacks` that all expand to
one thing if a particular query modifier is present, and to another
thing otherwise.
One of these macros looks for the `arena_cache` modifier:
- `query_if_arena`
Two of these macros look for the `return_result_from_ensure_ok`
modifier:
- `query_ensure_select`
- `ensure_ok_result`
Three of these macros look for the `separate_provide_extern` modifier:
- `local_key_if_separate_extern`
- `separate_provide_extern_decl`
- `separate_provide_extern_default`
(There is also `query_helper_param_ty!`, but it follows a different
pattern, and I will deal with it later.)
The "one thing"/"another thing" is different for every macro. For most
of them you can't see at the macro call site what the expansion will be;
you have to look at the macro definition. This is annoying.
This commit reduces the six macros into three macros:
- `if_arena_cache`
- `if_return_result_from_ensure_ok`
- `if_separate_provide_extern`
They all have the same form: they look for a modifier and then expand to
one *given* thing or the other *given* thing. You can see at the call
site the two possible expansions, which is much nicer. (Note:
`query_if_arena` already had this form.)
Ideally there would be a single macro instead of three, but I couldn't
work out a way to combine them.
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.
From `rustc_query_system` to `rustc_middle.` I put it in `graph.rs`,
it's one of two files that uses `QuerySideEffect` and seemed as good as
anywhere else.
Zalathar