Conversation
This comment has been minimized.
This comment has been minimized.
|
cc @oli-obk you may be interested in this, because this likely intersects with incremental AST lowering. |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
TY-aware delayed HIR lowering
|
|
||
| #[extension(trait ResolverAstLoweringExt)] | ||
| impl ResolverAstLowering { | ||
| impl ResolverAstLowering<'_> { |
There was a problem hiding this comment.
I went down a slightly different route: c98471e
That gets rid of the extension trait entirely and allows adding custom information that doesn't need to be crate global.
| providers.hir_module_items = map::hir_module_items; | ||
| providers.local_def_id_to_hir_id = |tcx, def_id| match tcx.hir_crate(()).owners[def_id] { | ||
| providers.get_delayed_child_owner = |_, _| MaybeOwner::Phantom; | ||
| providers.local_def_id_to_hir_id = |tcx, def_id| match tcx.hir_crate(()).owner(tcx, def_id) { |
There was a problem hiding this comment.
Just this change adding an owner method seems sth nice to do on main
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (a77c1ae): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 4.3%, secondary 3.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.7%, secondary 3.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 480.442s -> 479.934s (-0.11%) |
…o-hir-arena, r=petrochenkov Replace Box<[TraitCandidate]> with &'hir [TraitCandidate<'hir>] This PR allocates trait candidates on HIR arena and replaces `remove` with `get` in `ResolverAstLowering`. First step for rust-lang#153489. r? @petrochenkov
…, r=petrochenkov Replace Box<[TraitCandidate]> with &'hir [TraitCandidate<'hir>] This PR allocates trait candidates on HIR arena and replaces `remove` with `get` in `ResolverAstLowering`. First step for #153489. r? @petrochenkov
…o-hir-arena, r=petrochenkov Replace Box<[TraitCandidate]> with &'hir [TraitCandidate<'hir>] This PR allocates trait candidates on HIR arena and replaces `remove` with `get` in `ResolverAstLowering`. First step for rust-lang#153489. r? @petrochenkov
Rollup merge of #153494 - aerooneqq:boxed-trait-candidates-to-hir-arena, r=petrochenkov Replace Box<[TraitCandidate]> with &'hir [TraitCandidate<'hir>] This PR allocates trait candidates on HIR arena and replaces `remove` with `get` in `ResolverAstLowering`. First step for #153489. r? @petrochenkov
|
☔ The latest upstream changes (presumably #153541) made this pull request unmergeable. Please resolve the merge conflicts. |
This PR implements a prototype of TY-aware delayed HIR lowering. Part of #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_sigfor 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:
tcx.fn_sigtcx.fn_sigis_methodfunction now usestcx.associated_iteminstead of resolverget_external_genericsthat usestcx.generics_of. Generics for recursive delegations should also workDelegationIdsthat stored paths for recursive delegations is removed, we now use onlydelegee_idast_indexis no more usedNext steps
High level design overview
Queries
We store ids of delayed items to lower in
Cratestruct. During first stage of lowering, owners that correspond to delayed items are filled withMaybeOwner::Phantom.Next, we define two new queries:
lower_to_hir_delayedandget_delayed_child_owner.The first query is used when lowering known delayed owner.
The second is feeded with children that were obtained during lowering of a delayed owner (note that the result of lowering of a single
LocalDefIdis not a singleMaybeOwner, its a list of(LocalDefId, MaybeOwner)where the firstMaybeOwnercorresponds to delayedLocalDefIdand others to children that were obtained during lowering (i.e. generic params)). By defaultget_delayed_child_ownerreturnsMaybeOwner::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:lower_to_hir_delayedto beFxIndexMap<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 delayedLocalDefIdand its children.LocalDefIdsand their children. In this case there will be problems with delayed items that require information about other delayed items.By using such atomic 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 HIR lowering of someLocalDefIdshould not in any way access children of other, yet unlowered, delayedLocalDefId, they should firstly materialize it.Resolver for lowering
Currently the
resolver_for_loweringis stolen inlower_to_hirfunction, however we want to prolong its life until all delayedLocalDefIdsare materialized. For this purpose we borrowresolver_for_loweringinlower_to_hirand drop it after forcing materialization of all delayedLocalDefIdinrustc_interface::run_required_analyses.We also 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. Now we modify resolver in place as we steal it, however in this PR we borrow it so we need to split it.
AST index
Lowering uses an AST index. It is created in
lower_to_hirfunction and it references parts of AST. We want to avoid reindexing AST on every delayedLocalDefIdlowering, 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.