Overhaul `Erasable` impls
This PR removes many unused `Erasable` impls, converts many of the hand-written impls to macro-generated impls, and sorts the macro's inputs. This cuts over 200 lines of code and fixes three FIXME comments.
r? @petrochenkov
Some of the hand-written `Erasable` impls only match a single type in
practice. It's easier to just list the concrete types in
`impl_erasable_for_types_with_no_type_params!`.
Now that 'static lifetimes aren't used, a lot of the hand-written
`Erasable` impls can now be done with the macro. (The only ones that
can't are those with a generic type parameter, because `size_of`
doesn't work in that case.)
Also, `impl_erasable_for_single_lifetime_types!` isn't needed at all.
Unalign `PackedFingerprint` on all hosts, not just x86 and x86-64
Back in https://github.com/rust-lang/rust/pull/78646, `DepNode` was modified to store an unaligned `PackedFingerprint` instead of an 8-byte-aligned `Fingerprint`. That reduced the size of DepNode from 24 bytes to 17 bytes (nowadays 18 bytes), resulting in considerable memory savings in incremental builds.
See https://github.com/rust-lang/rust/pull/152695#issuecomment-3907091509 for a benchmark demonstrating the impact of *removing* that optimization.
At the time (and today), the unaligning was only performed on x86 and x86-64 hosts, because those CPUs are known to generally have low overhead for unaligned memory accesses. Hosts with other CPU architectures would continue to use an 8-byte-aligned fingerprint and a 24-byte DepNode.
Given the subsequent rise of aarch64 (especially on macOS) and other architectures, it's a shame that some commonly-used builds of rustc don't get those memory-size benefits, based on a decision made several years ago under different circumstances.
We don't have benchmarks to show the actual effect of unaligning DepNode fingerprints on various non-x86 hosts, but it seems very likely to be a good idea on Apple chips, and I have no particular reason to think that it will be catastrophically bad on other hosts. And we don't typically perform this kind of speculative pessimization in other parts of the compiler.
Don't store current-session side effects in `OnDiskCache`
This PR is a series of related cleanups to `OnDiskCache`, which is responsible for loading query return values (and side effects) that were serialized during the previous incremental-compilation session.
The primary change is to move the `current_side_effects` field out of OnDiskCache and into QuerySystem. That field was awkward because it was the only part of OnDiskCache state related to serializing the *current* compilation session, rather than loading values from the previous session.
The other commits should hopefully be straightforward.
r? nnethercote (or compiler)
Rename various query cycle things.
The existing names have bugged me for a while. Changes:
- `CycleError` -> `Cycle`. Because we normally use "error" for a `Diag` with `Level::Error`, and this type is just a precursor to that. Also, many existing locals of this type are already named `cycle`.
- `CycleError::cycle` -> `Cycle::frames`. Because it is a sequence of frames, and we want to avoid `Cycle::cycle` (and `cycle.cycle`).
- `cycle_error` -> `find_and_handle_cycle`. Because that's what it does. The existing name is just a non-descript noun phrase.
- `mk_cycle` -> `handle_cycle`. Because it doesn't make the cycle; the cycle is already made. It handles the cycle, which involves (a) creating the error, and (b) handling the error.
- `report_cycle` -> `create_cycle_error`. Because that's what it does: creates the cycle error (i.e. the `Diag`).
- `value_from_cycle_error` -> `handle_cycle_error_fn`. Because most cases don't produce a value, they just emit an error and quit. And the `_fn` suffix is for consistency with other vtable fns.
- `from_cycle_error` -> `handle_cycle_error`. A similar story for this module.
r? @petrochenkov
Every other part of `OnDiskCache` deals with loading information from the
_previous_ session, except for this one field.
Moving it out to `QuerySystem` makes more sense, because that's also where
query return values are stored (inside the caches in their vtables).
These lists can be considered part of the encoder state, and bundling them
inside the encoder is certainly more convenient than passing them around
separately.
The existing names have bugged me for a while. Changes:
- `CycleError` -> `Cycle`. Because we normally use "error" for a `Diag`
with `Level::Error`, and this type is just a precursor to that. Also,
many existing locals of this type are already named `cycle`.
- `CycleError::cycle` -> `Cycle::frames`. Because it is a sequence of
frames, and we want to avoid `Cycle::cycle` (and `cycle.cycle`).
- `cycle_error` -> `find_and_handle_cycle`. Because that's what it does.
The existing name is just a non-descript noun phrase.
- `mk_cycle` -> `handle_cycle`. Because it doesn't make the cycle; the
cycle is already made. It handles the cycle, which involves (a)
creating the error, and (b) handling the error.
- `report_cycle` -> `create_cycle_error`. Because that's what it does:
creates the cycle error (i.e. the `Diag`).
- `value_from_cycle_error` -> `handle_cycle_error_fn`. Because most
cases don't produce a value, they just emit an error and quit.
And the `_fn` suffix is for consistency with other vtable fns.
- `from_cycle_error` -> `handle_cycle_error`. A similar story for this
module.
Use enums to clarify `DepNodeColorMap` color marking
When a function's documentation has to explain the meaning of nested results and options, then it is often a good candidate for using a custom result enum instead.
This PR also renames `DepNodeColorMap::try_mark` to `try_set_color`, to make it more distinct from the similarly-named `DepGraph::try_mark_green`.
The difference is that `try_mark_green` is a higher-level operation that tries to determine whether a node _can_ be marked green, whereas `try_set_color` is a lower-level operation that actually records a colour for the node.
The updated docs for `try_set_color` also fix a typo: *atomicaly* → *atomically*.
r? nnethercote (or compiler)
Simplify find_attr! for HirId usage
Add a `HasAttrs<'tcx, Tcx>` trait to `rustc_hir` that allows `find_attr!` to accept `DefId`, `LocalDefId`, `OwnerId`, and `HirId` directly, instead of requiring callers to manually fetch the attribute slice first.
Before:
`find_attr!(tcx.hir_attrs(hir_id), SomeAttr)`
After:
`find_attr!(tcx, hir_id, SomeAttr)`
The trait is defined in `rustc_hir` with a generic `Tcx` parameter to avoid a dependency cycle (`rustc_hir` cannot depend on `rustc_middle`). The four concrete impls for `TyCtxt` are in `rustc_middle`.
Fixes https://github.com/rust-lang/rust/issues/153103
When a function's documentation has to explain the meaning of nested results
and options, then it is often a good candidate for using a custom result enum
instead.
This commit also renames `DepNodeColorMap::try_mark` to `try_set_color`, to
make it more distinct from the similarly-named `DepGraph::try_mark_green`.
The difference is that `try_mark_green` is a higher-level operation that tries
to determine whether a node _can_ be marked green, whereas `try_set_color` is a
lower-level operation that actually records a color for the node.
According to its comment, this query was only using `anon` to save a little bit
of work for a dep graph node that is expected to have no dependencies.
Benchmarks indicate that the perf loss, if any, is small enough to be justified
by the fact that we can now remove support for `anon` queries.
Adding `no_hash` appears to give marginally better perf results.
These queries appear to have been using `anon` for its side-effect of making
them ineligible for forcing.
According to their comments and also `tests/incremental/issue-61323.rs`, these
queries want to avoid forcing so that if a cycle does occur, the whole cycle
will be on the query stack for the cycle handler to find.
Guard patterns: lowering to THIR
This pr implements lowering of guard patterns to THIR
r? @Nadrieril
cc @max-niederman
Tracking issue: rust-lang/rust#129967
ty-aware delayed AST -> HIR lowering
This PR implements a prototype of ty-aware delayed AST -> HIR lowering. Part of rust-lang/rust#118212.
r? @petrochenkov
# Motivation
When lowering delegation in perfect scenario we would like to access the ty-level information, in particular, queries like `generics_of`, `type_of`, `fn_sig` for proper HIR generation with less hacks. For example, we do not want to generate more lifetimes than needed, because without ty-level queries we do not know which delegee's lifetimes are late-bound. Next, consider recursive delegations, for their proper support without ty we would have to either duplicate generics inheritance code in AST -> HIR lowering or create stubs for parts of the HIR and materialize them later. We already use those queries when interacting with external crates, however when dealing with compilation of a local crate we should use resolver for similar purposes. Finally, access to ty-level queries is expected to help supporting delegation to inherent impls, as we can not resolve such calls during AST -> HIR stage.
# Benefits
We eliminate almost all code that uses resolver in delegation lowering:
- Attributes inheritance is done without copying attributes from AST at resolve stage
- Fn signatures are obtained from `tcx.fn_sig`
- Param counts are also obtained from `tcx.fn_sig`
- `is_method` function now uses `tcx.associated_item` instead of resolver
- Generics are now inherited through `get_external_generics` that uses `tcx.generics_of`. Generics for recursive delegations should also work
- `DelegationIds` that stored paths for recursive delegations is removed, we now use only `delegee_id`
- Structs that were used for storing delegation-related information in resolver are almost fully removed
- `ast_index` is no more used
# Next steps
- Remove creating generic params through AST cloning, proper const types propagation
- Inherent impls
# High level design overview
## Queries
We store ids of delayed items to lower in `Crate` struct. During first stage of lowering, owners that correspond to delayed items are filled with `MaybeOwner::Phantom`.
Next, we define two new queries: `lower_delayed_owner`, `delayed_owner` and a function `force_delayed_owners_lowering`.
The first query is used when lowering known (which is in `delayed_ids`) delayed owner.
The second is fed with children that were obtained during lowering of a delayed owner (note that the result of lowering of a single `LocalDefId` is not a single `MaybeOwner`, its a list of `(LocalDefId, MaybeOwner)` where the first `MaybeOwner` corresponds to delayed `LocalDefId` and others to children that were obtained during lowering (i.e. generic params)). By default `delayed_owner` returns `MaybeOwner::Phantom`. As we do not want to predict the whole list of children which will be obtained after lowering of a single delayed item, we need to store those children somewhere. There are several options:
- Make the return type of `lower_delayed_owner` to be `FxIndexMap<LocalDefId, MaybeOwner>` and search children here. Search will be either linear or we can introduce a map somewhere which will track parent-child relations between a single delayed `LocalDefId` and its children.
- Try to make query that will lower all delayed items in a loop and return a complete map of all delayed `LocalDefIds` and their children. In this case there will be problems with delayed items that require information about other delayed items.
By using proposed queries we handle the second concern, and in case of acyclic dependencies between delayed ids it automatically works, moreover we use queries as cache for delayed `MaybeOwners`. The only invariant here is that queries which are invoked during delayed AST -> HIR lowering of some `LocalDefId` should not in any way access children of other, yet unlowered, delayed `LocalDefId`, they should firstly materialize it.
The `force_delayed_owners_lowering` forces lowering of all delayed items and now integrated in `hir_crate_items` query.
## Resolver for lowering
> ~Currently the `resolver_for_lowering` is stolen in `lower_to_hir` function, however we want to prolong its life until all delayed `LocalDefIds` are materialized. For this purpose we borrow `resolver_for_lowering` in `lower_to_hir` and drop it after forcing materialization of all delayed `LocalDefId` in `rustc_interface::run_required_analyses`.~
We split resolver for lowering into two parts: the first part is a readonly part that came to us from resolve, the second part is a mutable part that can be used to add or overwrite values in the readonly part. Such splitted resolver is used during delayed lowering, as we can't steal it.
## AST index
Lowering uses an AST index. It is created in `lower_to_hir` function and it references parts of AST. We want to avoid reindexing AST on every delayed `LocalDefId` lowering, however now it is not clear how to properly do it. As delayed HIR lowering is used only for delegation unstable feature it should not affect other use-cases of the compiler. But it will be reworked sooner or later.
Rewrite `query_ensure_result`.
It currently uses chaining which is concise but hard to read. There are up to four implicit matches occurring after the call to `execute_query_fn`.
- The first `map` on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- The second `map` on `Option<Result<T, ErrorGuaranteed>>`.
- The third `map` on `Result<T, ErrorGuaranteed>`.
- The `unwrap_or` on `Option<Result<(), ErrorGuaranteed>>`.
This commit rewrites it to use at most two matches.
- An explicit match on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- An explicit match on `Result<T, ErrorGuaranteed>`.
This is easier to read. It's also more efficient, though the code isn't hot enough for that to matter.
r? @Zalathar
fix inference variables leaking into HIR const literal lowering logic
Inference variables could leak into further const lowering logic
It ICEs when query system tries to cache `lit_to_const()` after its execution and panics, because inference variables are not hashable for some reason
Fixesrust-lang/rust#153524Fixesrust-lang/rust#153525
It currently uses chaining which is concise but hard to read. There are
up to four implicit matches occurring after the call to
`execute_query_fn`.
- The first `map` on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- The second `map` on `Option<Result<T, ErrorGuaranteed>>`.
- The third `map` on `Result<T, ErrorGuaranteed>`.
- The `unwrap_or` on `Option<Result<(), ErrorGuaranteed>>`.
This commit rewrites it to use at most two matches.
- An explicit match on `Option<Erased<Result<T, ErrorGuaranteed>>>`.
- An explicit match on `Result<T, ErrorGuaranteed>`.
This is easier to read. It's also more efficient, though the code isn't
hot enough for that to matter.
This is nitpicky, but the lack of a sensible order has been bugging me.
- Within `mod $name`, put all the typedefs together.
- After that:
- First, types definitions and their impls.
- Then the `TyCtxt*` impls, in a sensible order.
- Likewise, put `TyCtxt::at` before the similar methods.
- Also reflow some overly long lines.
All methods that accept `impl IntoQueryKey<_>` have been adjusted to
consistently call `into_query_key` before doing anything else.
When a function with a conversion trait calls another function with the same
conversion trait, doing the conversion eagerly can reduce the overall number of
instantiations.
Removing coherent and self-contained chunks of code from a catch-all "plumbing"
module is typically an improvement.
Giving the trait its own file also gets rid of the unhelpful `sealed` module
that it was previously in.
Jonathan Brouwer