Skip to content

delegation: simplify matches on FnKind, minor refactorings - #160853

Open
aerooneqq wants to merge 1 commit into
rust-lang:mainfrom
aerooneqq:delegation-fn-kind-matches
Open

delegation: simplify matches on FnKind, minor refactorings#160853
aerooneqq wants to merge 1 commit into
rust-lang:mainfrom
aerooneqq:delegation-fn-kind-matches

Conversation

@aerooneqq

@aerooneqq aerooneqq commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

This PR refactors matches on FnKind making them smaller and more concise, next in all matches except fn_kinds function we no longer panic on delegation to inherent impls. And some minor renamings/refactorings. First step for #160505.

Part of #118212.
r? @petrochenkov

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 10, 2026
@aerooneqq
aerooneqq force-pushed the delegation-fn-kind-matches branch 2 times, most recently from e9a04e2 to e680d29 Compare August 12, 2026 07:09
@petrochenkov petrochenkov added the F-fn_delegation `#![feature(fn_delegation)]` label Aug 18, 2026
Comment thread compiler/rustc_hir_analysis/src/delegation.rs Outdated
Comment thread compiler/rustc_hir_analysis/src/delegation.rs Outdated
Comment thread compiler/rustc_hir_analysis/src/delegation.rs Outdated
@petrochenkov

Copy link
Copy Markdown
Contributor

I'm generally skeptical about introduction of the extension trait.
It would be one thing if it was actually used as a trait (e.g. in bounds), or the methods were somehow "inherent" to TyCtxt.
But doing it just to use the method call syntax? I don't know.

Removing unnecessary lifetime parameters (#160853 (comment)) will make things nicer if the current setup without the extension trait is kept.

@petrochenkov

Copy link
Copy Markdown
Contributor

The logic behind the fn kind matching refactoring is also not very clear to me.
Marking the arms as unreachable is good, merging arms using the same logic like in get_delegation_parent_args_count_without_self is good, making the matches less exhaustive - maybe not so good for readability.

@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 Aug 18, 2026
@aerooneqq

Copy link
Copy Markdown
Contributor Author

Reverted creation of extension to TyCtxt, But doing it just to use the method call syntax? - yes, just esthetical thought that it would look better when match-like functions will be grouped together and calling them as method calls, not that important for now.

making the matches less exhaustive - maybe not so good for readability

I think exhaustive matching does not allow to see the general strategy of handling different FnKinds. Wildcard patterns allow to clearly see general case while all other arms are small and represent edge cases, where one of them is for unreachable!().

In get_delegation_parent_args_count_without_self it is better because the code became more compact and we have an explicit general case handling which uses tcx.generics_of, while in the original version tcx.generics_of was executed all the time at the beginning of the method. Plus the size of patterns in each arm reduced significantly which makes code more understandable.

In get_parent_and_inheritance_kind it is now evident what is the boolean parameter of WithParent variant means plus it is more semantically correct as in original version we returned true for (FnKind::Free, FnKind::Free). + arms' patterns size reduction.

In get_delegation_self_ty we clearly highlight that we create concrete type only when delegation is inside impl (trait or inherent), in all other cases we delegate core logic of creation of self_ty to create_self_param_position_kind, it is explicitly shown with wildcard pattern, while in the original exhaustive match it is not that clear.

So I like new matches more than old ones (wildcard patterns reduce thinking overhead which is needed when first-time looking at exhaustive pattern with 4 tuples of FnKind and trying to understand what all those cases have in common and where to add potential new one), and overall code size reduction is good I think.

@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 Aug 19, 2026
@petrochenkov

Copy link
Copy Markdown
Contributor

r=me after squashing commits.
@rustbot author
@bors delegate+

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

rust-bors Bot commented Aug 21, 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 Aug 21, 2026
@rustbot

rustbot commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

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

@aerooneqq

Copy link
Copy Markdown
Contributor Author

@bors squash

@rust-bors

This comment has been minimized.

* More concise matches on `FnKind`, some renamings
* Move most of utility functions to extensions
* Address review comments
* Revert "Move most of utility functions to extensions"
* Address review comments
@rust-bors

rust-bors Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

🔨 5 commits were squashed into d6e4a1d.

@rust-bors
rust-bors Bot force-pushed the delegation-fn-kind-matches branch from c5400fa to d6e4a1d Compare August 21, 2026 11:03
@aerooneqq

Copy link
Copy Markdown
Contributor Author

@bors r=petrochenkov

@rust-bors

rust-bors Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

📌 Commit d6e4a1d has been approved by petrochenkov

It is now in the queue for this repository.

@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 Aug 21, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 21, 2026
…es, r=petrochenkov

delegation: simplify matches on `FnKind`, minor refactorings

This PR refactors matches on `FnKind` making them smaller and more concise, next in all matches except `fn_kinds` function we no longer panic on delegation to inherent impls. And some minor renamings/refactorings. First step for rust-lang#160505.

Part of rust-lang#118212.
r? @petrochenkov
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 21, 2026
…es, r=petrochenkov

delegation: simplify matches on `FnKind`, minor refactorings

This PR refactors matches on `FnKind` making them smaller and more concise, next in all matches except `fn_kinds` function we no longer panic on delegation to inherent impls. And some minor renamings/refactorings. First step for rust-lang#160505.

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

Rollup of 16 pull requests

Successful merges:

 - #161259 (move some attribute related structs out of rustc_attr_ir)
 - #160853 (delegation: simplify matches on `FnKind`, minor refactorings)
 - #159899 ( `GenericArgs::types` triage + possible fixes)
 - #160459 (Use attribute parser for `deprecated` attribute checking)
 - #160813 (Optimize linked list iterator performance)
 - #161271 (doc: document safety requirements for core WTF-8)
 - #161317 (LLVM 24: configure float-abi via module flag)
 - #161320 (Only suggest `RUST_MIN_STACK` if maybe stack overflow)
 - #161369 (Add regression test for confusing lifetime error message issue)
 - #161393 (Configure LLM policy URL for triagebot)
 - #161403 (splat-fn-ptr-ptr-tuple.rs: add `let` to avoid UB)
 - #161409 (Add back `tests/rustdoc-gui/notable-trait.goml` test)
 - #161410 (Fix rustdoc remapping `documentation` scope documentation)
 - #161415 (Update expect messages in path docs to better follow guidelines)
 - #161438 (Change triagebot backport to ping T-libs-fcp)
 - #161442 (Add regression test for dead code on type alias used in impl self type)

Failed merges:

 - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`)
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 21, 2026
…es, r=petrochenkov

delegation: simplify matches on `FnKind`, minor refactorings

This PR refactors matches on `FnKind` making them smaller and more concise, next in all matches except `fn_kinds` function we no longer panic on delegation to inherent impls. And some minor renamings/refactorings. First step for rust-lang#160505.

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.

3 participants