Skip to content

Deduplication: Extract a shared helper function to remove duplicate associated const binding lowering - #155196

Open
LaneAsade wants to merge 2 commits into
rust-lang:mainfrom
LaneAsade:associated_const_binding_rhs_lowering
Open

Deduplication: Extract a shared helper function to remove duplicate associated const binding lowering#155196
LaneAsade wants to merge 2 commits into
rust-lang:mainfrom
LaneAsade:associated_const_binding_rhs_lowering

Conversation

@LaneAsade

@LaneAsadeLaneAsade commented Apr 12, 2026•
edited by jackh726
Loading

Copy link
Copy Markdown
Member

This PR aims to deduplicate the code the responsible for lowering the RHS of associated const bindings (found at bounds.rs and mod.rs) by creating a helper function lower_assoc_const_binding_rhs.

Related: #150621

@rustbot

rustbot commented Apr 12, 2026

Copy link
Copy Markdown
Collaborator

HIR ty lowering was modified

cc @fmease

@rustbotrustbot 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 Apr 12, 2026
@rustbot

rustbot commented Apr 12, 2026

Copy link
Copy Markdown
Collaborator

r? @jackh726

rustbot has assigned @jackh726.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler, types
  • compiler, types expanded to 69 candidates
  • Random selection from 12 candidates

@afurm

afurm commented Apr 12, 2026

Copy link
Copy Markdown

Nit: check_assoc_const_binding_type is called as bounds::check_assoc_const_binding_type at the mod.rs call site but as just check_assoc_const_binding_type (direct) at the bounds.rs call site — the helper normalises this to always call it as a method on self, which is correct, but worth verifying the two sites were always semantically equivalent (the FIXME comment in the old mod.rs code hints this was previously uncertain).

@LaneAsade

LaneAsade commented Apr 13, 2026

Copy link
Copy Markdown
MemberAuthor

As far as I know, the FIXME in the old mod.rs was about code duplication in lowering the RHS type and not about the equivalence of the two call sites. Even then, the two call sites are semantically equivalent so no worries there. Thank you for the feedback though!

@rust-bors

This comment has been minimized.

Sign in to view
@LaneAsade
LaneAsadeforce-pushed the associated_const_binding_rhs_lowering branch from 7458605 to c4c9ddcCompareApril 20, 2026 20:53
@rustbot

rustbot commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rust-bors

This comment has been minimized.

Sign in to view
jackh726
jackh726 requested changes Apr 23, 2026
View reviewed changes

@jackh726jackh726 left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIXME no longer relevant, then r=me

View changes since this review

Comment threadcompiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs Outdated
@rustbotrustbot 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 Apr 23, 2026
@rustbot

rustbot commented Apr 23, 2026

Copy link
Copy Markdown
Collaborator

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

@jackh726

jackh726 commented Apr 23, 2026

Copy link
Copy Markdown
Member

Btw, as an aside: it's not really needed or useful to include the code of the helpful function in the PR description - I went ahead and removed it.

@LaneAsade

LaneAsade commented Apr 25, 2026

Copy link
Copy Markdown
MemberAuthor

Thank you for the tip. I will keep that in mind next time.

@LaneAsade

LaneAsade commented Apr 25, 2026•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

@bors r=jackh726

@rustbot

rustbot commented Apr 25, 2026

Copy link
Copy Markdown
Collaborator

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

@LaneAsade

LaneAsade commented Apr 25, 2026

Copy link
Copy Markdown
MemberAuthor

@rustbot ready

@rustbotrustbot 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 Apr 25, 2026
@jackh726

jackh726 commented May 22, 2026

Copy link
Copy Markdown
Member

Apologies for the review delay :( now needs a rebase

@jackh726jackh726 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 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jackh726jackh726jackh726 requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@jackh726jackh726

Labels

S-waiting-on-authorStatus: This is awaiting some action (such as code changes or more information) from the author.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@LaneAsade@rustbot@afurm@jackh726