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

[sanidhyasin]

Fixes #144378.

The bug

When detecting unused parentheses around a trait-object / impl Trait bound, UnusedParens::check_ty builds the "remove these parentheses" suggestion by trimming the first and last byte of poly_trait_ref.span:

let spans = (!s.from_expansion()).then(|| {
    (
        s.with_hi(s.lo() + rustc_span::BytePos(1)),
        s.with_lo(s.hi() - rustc_span::BytePos(1)),
    )
});

poly_trait_ref.parens == Yes only records that the parser saw parentheses around the bound — it does not guarantee that s points at them in the source. A proc-macro can synthesize the parentheses while reusing an unrelated span from its input (the reproducer in the issue does this via modular-bitfield). On such a span the byte trimming:

• produces an invalid suggestion, e.g. rewriting a field val: u8 into al: u, and
• can ICE when the reused span starts or ends on a multibyte character (slicing mid-char).

This is a regression from #142237, which introduced the bounds-detection path.

The fix

Only emit the lint when the source text at the span really is wrapped in parentheses, mirroring the robustness of the existing TyKind::Paren arm. The (/) are ASCII, so once the snippet is confirmed to start/end with them the existing byte arithmetic is correct. Spans for genuine source parentheses are unchanged, so the existing unused_parens UI tests keep their exact output.

Test

tests/ui/lint/unused/unused-parens-trait-obj-proc-macro-144378.rs uses a small proc-macro that emits &dyn (Send) while re-using the span of a multibyte identifier. Before this change it ICEd; now it compiles cleanly (check-pass).

[rustbot]

r? @mejrs

rustbot has assigned @mejrs.
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 19 candidates

[Kivooeo]

Hi, thank your for your PR. I have a question regarding LLM tools, was and how LLMs been used to create this PR?

[sanidhyasin]

Hi, thank your for your PR. I have a question regarding LLM tools, was and how LLMs been used to create this PR?

Hey, thanks for taking a look!

Yeah, I used Claude to help investigate the issue and find the PR that introduced the regression. It also helped me put together the fix and test.

But I did go through it all myself and understand what's happening. The old code assumes the span always includes the surrounding parentheses, which isn't always true when a proc-macro reuses a span. In those cases, trimming the first and last byte can end up mangling the source and even ICE on multibyte characters.

The fix just checks that the source actually starts and ends with parentheses before trying to lint it.

Happy to change the approach if you think there's a better way to handle it.

[mejrs]

You can hit this with parser recovery as well by the way:

const _: &dyn（Send）= &();

Happy to change the approach if you think there's a better way to handle it.

Using BytePos(1) in this way is rather suspicious, It'd nice if we could avoid using it but I don't see an easy way.

[sanidhyasin]

Hey, thanks for the example!

Turns out the current check already handles that case. Those are fullwidth parens （ ）, not ASCII ( ) — the lexer quietly rewrites them into (/) tokens for recovery, but the span still covers the original multi-byte characters. The check looks at the actual source text and only proceeds if it starts with ( and ends with ); fullwidth （ isn't the same character as (, so it bails out instead of slicing into the middle of a multi-byte char (which was the crash). The lexer also already emits its own "use ( instead of （" error on that line, so we're not hiding anything by skipping the lint there.

On BytePos(1) — yeah, agreed it looked off. The field's own doc even says "the first and last character of span are parens", and the old code basically assumed one character is one byte. Now that the check confirms the parens are real ASCII ( ) before we trim, and those are always exactly one byte, the +1/-1 is sound. I expanded the comment there to spell this out, covering both the proc-macro and the Unicode-paren cases.

I did look at dropping BytePos entirely — the TyKind::Paren arm does it nicely by using the inner type's span instead of counting bytes — but a parenthesized bound can also carry for<'a> / modifiers, so there's no single inner span to lean on. The text check was the most reliable thing I could find.

I also tightened the regression test while I was here: it was reusing the span of naïve, whose first and last chars happen to be single-byte, so it didn't actually hit the ICE. It now reuses the span of é (a two-byte char), which does reproduce the original crash before the fix.

[sanidhyasin]

Sorry for the noise — I accidentally force-pushed this branch from a shallow clone, which dropped its shared history with master and auto-closed this PR (GitHub won't let it be reopened after that). The branch is fixed and the work continues in #157692, with your feedback addressed. cc @mejrs