close
Skip to content

Fix unused_parens for pinned reference patterns#156089

Open
P8L1 wants to merge 1 commit into
rust-lang:mainfrom
P8L1:fix-unused-parens-pinned-patterns
Open

Fix unused_parens for pinned reference patterns#156089
P8L1 wants to merge 1 commit into
rust-lang:mainfrom
P8L1:fix-unused-parens-pinned-patterns

Conversation

@P8L1
Copy link
Copy Markdown
Contributor

@P8L1 P8L1 commented May 2, 2026 •
edited
Loading

The unused_parens lint previously treated pinned shared-reference patterns like plain shared-reference patterns when deciding whether removing parentheses could change the meaning of a leading mut binding.

That was too conservative for pinned reference patterns such as &pin const (mut x). The pin const part belongs to the outer reference pattern, so removing the inner parentheses still leaves mut x as the subpattern.

This updates the ambiguity check so the leading-mut suppression only applies to plain shared reference patterns. Pinned reference patterns can now receive correct rustfix suggestions.

The patch also broadens the binding check from only plain mut bindings to all binding modes whose binding mutability is Mutability::Mut, covering forms such as mut ref pin const.

@rustbot label F-pin_ergonomics

@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 May 2, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 2, 2026

r? @nnethercote

rustbot has assigned @nnethercote.
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
  • compiler expanded to 73 candidates
  • Random selection from 21 candidates

@Kivooeo
Copy link
Copy Markdown
Member

Kivooeo commented May 3, 2026

same as here: #156087 (comment)

@rustbot author

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 3, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 3, 2026

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 May 3, 2026
@P8L1
Copy link
Copy Markdown
Contributor Author

P8L1 commented May 3, 2026

same as here: #156087 (comment)

@rustbot author

I gated the pinned-pattern unused-parens rustfix path on pin_ergonomics, so it is only emitted for crates that opted into the feature. I also left a FIXME next to the gate saying to remove it once pin_ergonomics is stabilized.

@P8L1
Copy link
Copy Markdown
Contributor Author

P8L1 commented May 3, 2026

@rustbot ready

Addressed the review feedback by gating the pinned-pattern unused_parens rustfix path on pin_ergonomics, so it is only emitted for crates that opted into the feature.

I also added a FIXME next to the gate saying to remove it once pin_ergonomics is stabilized.

@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 May 3, 2026
@P8L1
Copy link
Copy Markdown
Contributor Author

P8L1 commented May 3, 2026

r? @Kivooeo

@rustbot rustbot assigned Kivooeo and unassigned nnethercote May 3, 2026
@P8L1
Copy link
Copy Markdown
Contributor Author

P8L1 commented May 4, 2026

@Kivooeo Please review this as well

@Kivooeo
Copy link
Copy Markdown
Member

Kivooeo commented May 4, 2026

it's been only two days since it's open, review may take up to two weeks

since i'm pretty busy with other stuff i'd ask you to wait, because it may take a little longer

@P8L1
Copy link
Copy Markdown
Contributor Author

P8L1 commented May 16, 2026

Hey @Kivooeo, just wanted to gently bump this since it’s been about two weeks.

I think I’ve addressed the previous feedback, but please let me know if there’s anything else you’d like changed.

Thanks!

Kivooeo
Kivooeo reviewed May 17, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@Kivooeo Kivooeo left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

looks fine to me, but some nits

View changes since this review

Comment thread compiler/rustc_lint/src/unused.rs Outdated
Comment thread compiler/rustc_lint/src/unused.rs Outdated
Comment thread tests/ui/lint/unused-parens-pinned-reference-patterns.fixed
@rust-bors

This comment has been minimized.

Sign in to view
@P8L1 P8L1 force-pushed the fix-unused-parens-pinned-patterns branch from 696ca4a to 582523b Compare May 17, 2026 15:21
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 17, 2026

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.

@P8L1
Copy link
Copy Markdown
Contributor Author

P8L1 commented May 17, 2026 •
edited
Loading

@Kivooeo ready
I addressed all the nits and squashed to 1 commit

@P8L1 P8L1 force-pushed the fix-unused-parens-pinned-patterns branch from 582523b to ffe1dba Compare May 17, 2026 19:13
@rustbot rustbot added the F-pin_ergonomics `#![feature(pin_ergonomics)]` label May 22, 2026
@P8L1 P8L1 requested a review from Kivooeo May 23, 2026 15:43
@P8L1
Copy link
Copy Markdown
Contributor Author

P8L1 commented May 28, 2026 •
edited
Loading

@Kivooeo Hi, sorry for the ping, it's been another 2 weeks. Is there a chance you could have another look? I have addressed the nits

@Kivooeo
Copy link
Copy Markdown
Member

Kivooeo commented May 29, 2026

I'll review today-tomorrow

@Kivooeo
Copy link
Copy Markdown
Member

Kivooeo commented May 30, 2026

Got very busy today, will review tomorrow 100%, sorry for a delay

@P8L1
Copy link
Copy Markdown
Contributor Author

P8L1 commented May 30, 2026

Got very busy today, will review tomorrow 100%, sorry for a delay

Ok thanks so much, I appreciate you!

Kivooeo
Kivooeo reviewed May 31, 2026
View reviewed changes
Comment thread compiler/rustc_lint/src/unused.rs
PatKind::Or(..) if avoid_or => return,
// Avoid `mut x` and `mut x @ p` if we should:
PatKind::Ident(BindingMode::MUT, ..) if avoid_mut => {
// Avoid bindings that start with `mut`, like `mut x`, `mut x @ p`,
Copy link
Copy Markdown
Member

@Kivooeo Kivooeo May 31, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

question regarding this comment and then merge: "start with mut" wording, what about patterns like ref mut for example? is that going to work?

View changes since the review

Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 May 31, 2026

Choose a reason for hiding this comment

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

Yes, "ref mut" should still work here.

This branch is checking the binding’s own mutability ("BindingMode(, Mutability::Mut)"), not the mutability in "ByRef". "ref mut" is represented as "BindingMode(ByRef::Yes(, Mutability::Mut), Mutability::Not)", so it does not match this case.

The wording is a bit imprecise though. I can reword the comment to say “bindings with binding mutability” instead of “bindings that start with "mut"”.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kivooeo Kivooeo Kivooeo left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@Kivooeo Kivooeo

Labels

F-pin_ergonomics `#![feature(pin_ergonomics)]` 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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@P8L1 @rustbot @Kivooeo @nnethercote