Skip to content

Don't emit unused_parens suggestion for proc-macro-synthesized parens around bounds#157692

Open
sanidhyasin wants to merge 1 commit into
rust-lang:mainfrom
sanidhyasin:fix-unused-parens-proc-macro-span
Open

Don't emit unused_parens suggestion for proc-macro-synthesized parens around bounds#157692
sanidhyasin wants to merge 1 commit into
rust-lang:mainfrom
sanidhyasin:fix-unused-parens-proc-macro-span

Conversation

@sanidhyasin

@sanidhyasin sanidhyasin commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Supersedes #157662, which got auto closed after I force pushed the branch and github won't let me reopen it. Fixes #144378.

When the unused_parens lint removes the parentheses around a trait bound, it assumed the bound's span starts and ends with those parentheses, so it just trimmed one byte off each end to build the suggestion. That isn't always true. A proc macro can give the bound a span that doesn't point at parentheses at all, and then trimming a byte produces a broken suggestion, or crashes when that byte lands in the middle of a multibyte character.

So now we only remove the parens when the source text at that span actually starts with ( and ends with ). That also handles the unicode parens case you mentioned, since isn't an ASCII ( and just gets skipped.

I added a test that reproduces the crash with a proc macro.

r? @mejrs

`poly_trait_ref.parens` only records that the parser saw parentheses
around a trait-object/impl-trait bound; it does not guarantee that the
bound's span actually points at those parentheses in the source. A
proc-macro can synthesize the parentheses while reusing an unrelated span
from its input, so the span may not be wrapped in parentheses at all.

Previously the lint unconditionally trimmed the first and last byte of the
span to build the "remove these parentheses" suggestion. On such reused
spans this produced an invalid suggestion (e.g. rewriting a field
`val: u8` to `al: u`) and could even ICE when the span started or ended on
a multibyte character.

Only emit the lint when the source text at the span really is wrapped in
parentheses.
@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 Jun 10, 2026
@mejrs

mejrs commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Regarding @Kivooeo's question about whether you used LLM tools; you did not disclose whether the PR description and comments are LLM generated or not. It reads like they are. I'd prefer you write them yourself and be concise and to the point. You include a lot of useless details that make it annoying to read.

@sanidhyasin

Copy link
Copy Markdown
Contributor Author

Regarding @Kivooeo's question about whether you used LLM tools; you did not disclose whether the PR description and comments are LLM generated or not. It reads like they are. I'd prefer you write them yourself and be concise and to the point. You include a lot of useless details that make it annoying to read.

I used an LLM to help write and format the description and comments. I've rewritten the description myself and trimmed it down, and I'll keep it concise going forward. Thanks.

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

Labels

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

Development

Successfully merging this pull request may close these issues.

unused_parens generates invalid code

3 participants