Skip to content

delegation: refactor AST -> HIR lowering#158434

Open
aerooneqq wants to merge 1 commit into
rust-lang:mainfrom
aerooneqq:delegation-refactoring
Open

delegation: refactor AST -> HIR lowering#158434
aerooneqq wants to merge 1 commit into
rust-lang:mainfrom
aerooneqq:delegation-refactoring

Conversation

@aerooneqq

@aerooneqq aerooneqq commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

View all comments

After a lot of new features were added to delegation the code became a bit worse, as resolution and lowering logic is mixed in delegation.rs. In this file the output of resolution step of delegation is transferred as function params (in some functions up to 7 or 8 params), which is not good looking for me.

As I mentioned in the previous paragraph delegation AST -> HIR lowering can be split into two phases:

  1. Resolution - we collect all information about signature, signature parent context, delegation parent, delegation generics. Then this information is used in AST -> HIR lowering,
  2. AST -> HIR lowering - we generate everything based on the output of resolution.

This PR explicitly splits those two phases: as a first step we collect all needed information into DelegationResolution struct and then use it during AST -> HIR lowering. The delegation.rs file is moved into delegation folder and renamed to mod.rs, the final file structure is as follows:

  • mod.rs - contains lowering logic for everything but generics,
  • generics.rs - contains logic for resolution and uplifting of generic params, placed in a separate file as it is a significant and independent part of lowering,
  • attributes.rs - contains relatively small piece of logic which connects to attributes inheritance,
  • resolution.rs - contains logic for resolution of everything but generics.

As resolution of delegation and generics logically does not depend on LoweringContext, as from its name LoweringContext contains many additional information which is used for lowering, I've created LoweringContextForResolution trait which abstracts operations which are used during delegation resolution.

Benefits of this PR:

  • Resolution information is grouped into single struct, is extracted from AST -> HIR lowering which leads to better transfer of resolution across lowering (through a single struct, not through many arguments) and enhanced understanding of which information we use for delegation lowering,
  • There is now a single path for generating error delegation - if an error was returned from delegation resolution,
  • Refactorings of uplift_delegation_generics which was creating generic params for delegation made the code more understandable.

Part of #118212.
r? @petrochenkov

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 26, 2026
@aerooneqq

aerooneqq commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Now this PR is based on #158397, so it is draft until #158397 is merged, but feedback is anyway welcomed.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-bors

This comment has been minimized.

@aerooneqq aerooneqq force-pushed the delegation-refactoring branch from d6dd2be to c2d7c9c Compare June 29, 2026 12:03
@rust-bors

This comment has been minimized.

@aerooneqq aerooneqq force-pushed the delegation-refactoring branch from ae9c536 to faa250b Compare July 1, 2026 06:56
@rust-bors

This comment has been minimized.

@aerooneqq aerooneqq force-pushed the delegation-refactoring branch from faa250b to 002fc94 Compare July 2, 2026 05:42
@aerooneqq aerooneqq marked this pull request as ready for review July 2, 2026 05:45
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 2, 2026
@petrochenkov petrochenkov added the F-fn_delegation `#![feature(fn_delegation)]` label Jul 2, 2026
@petrochenkov

Copy link
Copy Markdown
Contributor

Could you split this into 2 commits - one moving code around (to different files, or within files), and the second one doing the code changes/refactorings?
@rustbot author

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2026
@rustbot

rustbot commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 2, 2026
@aerooneqq aerooneqq force-pushed the delegation-refactoring branch from 67f05b8 to 2196005 Compare July 2, 2026 11:11
@aerooneqq

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 2, 2026
Comment thread compiler/rustc_ast_lowering/src/delegation/attributes.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/resolution.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/resolution.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/mod.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/mod.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/mod.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/generics.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/generics.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/generics.rs Outdated
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 2, 2026
@aerooneqq

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 2, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2026
Comment thread compiler/rustc_ast_lowering/src/delegation/resolution.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/resolution.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/resolution.rs Outdated
Comment thread compiler/rustc_ast_lowering/src/delegation/generics.rs Outdated
@petrochenkov

Copy link
Copy Markdown
Contributor

r=me after addressing the remaining comments and squashing the refactoring commits.
@rustbot author
@bors delegate+

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2026
@rust-bors

rust-bors Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

✌️ @aerooneqq, you can now approve this pull request!

If @petrochenkov told you to "r=me" after making some further change, then please make that change and post @bors r=petrochenkov.

View changes since this delegation.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 2, 2026
@rust-log-analyzer

This comment has been minimized.

@aerooneqq aerooneqq force-pushed the delegation-refactoring branch from aec2af7 to d42cca5 Compare July 2, 2026 19:40
@aerooneqq

Copy link
Copy Markdown
Contributor Author

@bors r=petrochenkov

@rust-bors

rust-bors Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

📌 Commit d42cca5 has been approved by petrochenkov

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 2, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 3, 2026
…r=petrochenkov

delegation: refactor AST -> HIR lowering

After a lot of new features were added to delegation the code became a bit worse, as resolution and lowering logic is mixed in `delegation.rs`. In this file the output of resolution step of delegation is transferred as function params (in some functions up to 7 or 8 params), which is not good looking for me.

As I mentioned in the previous paragraph delegation AST -> HIR lowering can be split into two phases:

1. Resolution - we collect all information about signature, signature parent context, delegation parent, delegation generics. Then this information is used in AST -> HIR lowering,
2. AST -> HIR lowering - we generate everything based on the output of resolution.

This PR explicitly splits those two phases: as a first step we collect all needed information into `DelegationResolution` struct and then use it during AST -> HIR lowering. The `delegation.rs` file is moved into `delegation` folder and renamed to `mod.rs`, the final file structure is as follows:

- `mod.rs` - contains lowering logic for everything but generics,
- `generics.rs` - contains logic for resolution and uplifting of generic params, placed in a separate file as it is a significant and independent part of lowering,
- `attributes.rs` - contains relatively small piece of logic which connects to attributes inheritance,
- `resolution.rs` - contains logic for resolution of everything but generics.

As resolution of delegation and generics logically does not depend on `LoweringContext`, as from its name `LoweringContext` contains many additional information which is used for lowering, I've created `LoweringContextForResolution` trait which abstracts operations which are used during delegation resolution.

Benefits of this PR:

- Resolution information is grouped into single struct, is extracted from AST -> HIR lowering which leads to better transfer of resolution across lowering (through a single struct, not through many arguments) and enhanced understanding of which information we use for delegation lowering,
- There is now a single path for generating error delegation - if an error was returned from delegation resolution,
- Refactorings of `uplift_delegation_generics` which was creating generic params for delegation made the code more understandable.

Part of rust-lang#118212.
r? @petrochenkov
rust-bors Bot pushed a commit that referenced this pull request Jul 5, 2026
Rollup of 18 pull requests

Successful merges:

 - #158692 (Add release notes for 1.96.1)
 - #134021 (Implement `IntoIterator` for `[&[mut]] Box<[T; N], A>`)
 - #152860 (Port the `without_debuginfo` test from `backtrace-rs` to the testsuite)
 - #155932 (MIR Call terminator: evaluate destination place before arguments)
 - #156777 (Add -Zautodiff_post_passes flag to limit which llvm passes to run after enzyme to make autodiff tests more robust)
 - #157151 (JSON target specs: remove 'x86-softfloat' compatibility alias)
 - #157835 (expand free alias types in the auto-trait orphan check)
 - #158377 (add `-Zforce-intrinsic-fallback` flag)
 - #158434 (delegation: refactor AST -> HIR lowering)
 - #158552 (make some tidy errors around python easier to understand)
 - #158624 (borrowck: Introduce BlameConstraint::to_obligation_cause_from_path())
 - #158704 (Optimize `ArrayChunks::try_rfold` with `DoubleEndedIterator::next_chunk_back`)
 - #158711 (library: Comment on libtest's dicey internal soundness)
 - #158539 (Move `SizeHint` and `IoHandle` to `core::io`)
 - #158659 (refactor the normalization in `coerce_shared_info`)
 - #158689 (resolver: don't use `Finalize` when resolving visibilities during AST expansion)
 - #158698 (Update TypeVisitable implementation)
 - #158706 (Tweaks to MIR building scope API)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 5, 2026
…r=petrochenkov

delegation: refactor AST -> HIR lowering

After a lot of new features were added to delegation the code became a bit worse, as resolution and lowering logic is mixed in `delegation.rs`. In this file the output of resolution step of delegation is transferred as function params (in some functions up to 7 or 8 params), which is not good looking for me.

As I mentioned in the previous paragraph delegation AST -> HIR lowering can be split into two phases:

1. Resolution - we collect all information about signature, signature parent context, delegation parent, delegation generics. Then this information is used in AST -> HIR lowering,
2. AST -> HIR lowering - we generate everything based on the output of resolution.

This PR explicitly splits those two phases: as a first step we collect all needed information into `DelegationResolution` struct and then use it during AST -> HIR lowering. The `delegation.rs` file is moved into `delegation` folder and renamed to `mod.rs`, the final file structure is as follows:

- `mod.rs` - contains lowering logic for everything but generics,
- `generics.rs` - contains logic for resolution and uplifting of generic params, placed in a separate file as it is a significant and independent part of lowering,
- `attributes.rs` - contains relatively small piece of logic which connects to attributes inheritance,
- `resolution.rs` - contains logic for resolution of everything but generics.

As resolution of delegation and generics logically does not depend on `LoweringContext`, as from its name `LoweringContext` contains many additional information which is used for lowering, I've created `LoweringContextForResolution` trait which abstracts operations which are used during delegation resolution.

Benefits of this PR:

- Resolution information is grouped into single struct, is extracted from AST -> HIR lowering which leads to better transfer of resolution across lowering (through a single struct, not through many arguments) and enhanced understanding of which information we use for delegation lowering,
- There is now a single path for generating error delegation - if an error was returned from delegation resolution,
- Refactorings of `uplift_delegation_generics` which was creating generic params for delegation made the code more understandable.

Part of rust-lang#118212.
r? @petrochenkov
rust-bors Bot pushed a commit that referenced this pull request Jul 5, 2026
…uwer

Rollup of 20 pull requests

Successful merges:

 - #158692 (Add release notes for 1.96.1)
 - #134021 (Implement `IntoIterator` for `[&[mut]] Box<[T; N], A>`)
 - #155932 (MIR Call terminator: evaluate destination place before arguments)
 - #155989 (Update `transmute_copy` to ub_checks and `?Sized`)
 - #156777 (Add -Zautodiff_post_passes flag to limit which llvm passes to run after enzyme to make autodiff tests more robust)
 - #157151 (JSON target specs: remove 'x86-softfloat' compatibility alias)
 - #157835 (expand free alias types in the auto-trait orphan check)
 - #157857 (Stabilize `#[my_macro] mod foo;` (part of `proc_macro_hygiene`))
 - #158377 (add `-Zforce-intrinsic-fallback` flag)
 - #158434 (delegation: refactor AST -> HIR lowering)
 - #158552 (make some tidy errors around python easier to understand)
 - #158624 (borrowck: Introduce BlameConstraint::to_obligation_cause_from_path())
 - #158704 (Optimize `ArrayChunks::try_rfold` with `DoubleEndedIterator::next_chunk_back`)
 - #158711 (library: Comment on libtest's dicey internal soundness)
 - #158751 (rustdoc: Fix crash when trying to inline foreign item which cannot have attributes)
 - #158539 (Move `SizeHint` and `IoHandle` to `core::io`)
 - #158659 (refactor the normalization in `coerce_shared_info`)
 - #158689 (resolver: don't use `Finalize` when resolving visibilities during AST expansion)
 - #158698 (Update TypeVisitable implementation)
 - #158706 (Tweaks to MIR building scope API)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 5, 2026
…r=petrochenkov

delegation: refactor AST -> HIR lowering

After a lot of new features were added to delegation the code became a bit worse, as resolution and lowering logic is mixed in `delegation.rs`. In this file the output of resolution step of delegation is transferred as function params (in some functions up to 7 or 8 params), which is not good looking for me.

As I mentioned in the previous paragraph delegation AST -> HIR lowering can be split into two phases:

1. Resolution - we collect all information about signature, signature parent context, delegation parent, delegation generics. Then this information is used in AST -> HIR lowering,
2. AST -> HIR lowering - we generate everything based on the output of resolution.

This PR explicitly splits those two phases: as a first step we collect all needed information into `DelegationResolution` struct and then use it during AST -> HIR lowering. The `delegation.rs` file is moved into `delegation` folder and renamed to `mod.rs`, the final file structure is as follows:

- `mod.rs` - contains lowering logic for everything but generics,
- `generics.rs` - contains logic for resolution and uplifting of generic params, placed in a separate file as it is a significant and independent part of lowering,
- `attributes.rs` - contains relatively small piece of logic which connects to attributes inheritance,
- `resolution.rs` - contains logic for resolution of everything but generics.

As resolution of delegation and generics logically does not depend on `LoweringContext`, as from its name `LoweringContext` contains many additional information which is used for lowering, I've created `LoweringContextForResolution` trait which abstracts operations which are used during delegation resolution.

Benefits of this PR:

- Resolution information is grouped into single struct, is extracted from AST -> HIR lowering which leads to better transfer of resolution across lowering (through a single struct, not through many arguments) and enhanced understanding of which information we use for delegation lowering,
- There is now a single path for generating error delegation - if an error was returned from delegation resolution,
- Refactorings of `uplift_delegation_generics` which was creating generic params for delegation made the code more understandable.

Part of rust-lang#118212.
r? @petrochenkov
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

F-fn_delegation `#![feature(fn_delegation)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants