Skip to content

parser: fix parsing of trait bound polarity and for-binders #20417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 10, 2025

Conversation

npmccallum
Copy link
Contributor

The rustc AST allows both for<> binders and ? polarity modifiers in trait bounds, but they are parsed in a specific order and validated for correctness:

  1. for<> binder is parsed first.
  2. Polarity modifiers (?, !) are parsed second.
  3. The parser validates that binders and polarity modifiers do not conflict:
if let Some(binder_span) = binder_span {
    match modifiers.polarity {
        BoundPolarity::Maybe(polarity_span) => {
            // Error: "for<...> binder not allowed with ? polarity"
        }
    }
}

This implies:

  • for<> ?Sized → Valid syntax. Invalid semantics.
  • ?for<> Sized → Invalid syntax.

However, rust-analyzer incorrectly had special-case logic that allowed ?for<> as valid syntax. This fix removes that incorrect special case, making rust-analyzer reject ?for<> Sized as a syntax error, matching rustc behavior.

This has caused confusion in other crates (such as syn) which rely on these files to implement correct syntax evaluation.

The rustc AST allows both `for<>` binders and `?` polarity
modifiers in trait bounds, but they are parsed in a specific
order and validated for correctness:

  1. `for<>` binder is parsed first.
  2. Polarity modifiers (`?`, `!`) are parsed second.
  3. The parser validates that binders and polarity modifiers
     do not conflict:

```rust
if let Some(binder_span) = binder_span {
    match modifiers.polarity {
        BoundPolarity::Maybe(polarity_span) => {
            // Error: "for<...> binder not allowed with ? polarity"
        }
    }
}
```

This implies:

- `for<> ?Sized` → Valid syntax. Invalid semantics.
- `?for<> Sized` → Invalid syntax.

However, rust-analyzer incorrectly had special-case logic that
allowed `?for<>` as valid syntax. This fix removes that incorrect
special case, making rust-analyzer reject `?for<> Sized` as a
syntax error, matching rustc behavior.

This has caused confusion in other crates (such as syn) which
rely on these files to implement correct syntax evaluation.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 10, 2025
npmccallum added a commit to npmccallum/syn that referenced this pull request Aug 10, 2025
I have fixed the upstream bug which led to this mistake:

rust-lang/rust-analyzer#20417
npmccallum added a commit to npmccallum/syn that referenced this pull request Aug 10, 2025
I have fixed the upstream bug which led to this mistake:

rust-lang/rust-analyzer#20417
@ChayimFriedman2
Copy link
Contributor

ChayimFriedman2 commented Aug 10, 2025

Interesting, the current parsing used to work (and even added specifically in #12444), but that changed in rust-lang/rust#127054.

Copy link
Contributor

@ChayimFriedman2 ChayimFriedman2 left a comment

Choose a reason for hiding this comment

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

Thanks!

@ChayimFriedman2 ChayimFriedman2 added this pull request to the merge queue Aug 10, 2025
@ChayimFriedman2
Copy link
Contributor

BTW, in what way does syn rely on r-a's parsing?

Merged via the queue into rust-lang:master with commit 29cb5d4 Aug 10, 2025
15 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants