Don't emit unused_parens suggestion for proc-macro-synthesized parens around bounds#157692
Don't emit unused_parens suggestion for proc-macro-synthesized parens around bounds#157692sanidhyasin wants to merge 1 commit into
unused_parens suggestion for proc-macro-synthesized parens around bounds#157692Conversation
`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
added
S-waiting-on-review
|
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. |
| // In both cases trimming a single byte off each end would corrupt | ||
| // the suggestion or even ICE by slicing through a multibyte char. | ||
| // So only lint when the source really is wrapped in ASCII parens; | ||
| // those are exactly one byte each, which makes the trimming sound. |
There was a problem hiding this comment.
Please pare down this entire comment as well. It can just be a single sentence like "check that there's actually braces where we are expecting them to be, in case a macro does weird things with spans or parser recovery happens"
mejrs
added
S-waiting-on-author
3 participants
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