1
Fork 0

Rollup merge of #127054 - compiler-errors:bound-ordering, r=fmease

Reorder trait bound modifiers *after* `for<...>` binder in trait bounds

This PR suggests changing the grammar of trait bounds from:

```
[CONSTNESS] [ASYNCNESS] [?] [BINDER] [TRAIT_PATH]

const async ? for<'a> Sized
```

to

```
([BINDER] [CONSTNESS] [ASYNCNESS] | [?]) [TRAIT_PATH]
```

i.e., either

```
? Sized
```

or

```
for<'a> const async Sized
```

(but not both)

### Why?

I think it's strange that the binder applies "more tightly" than the `?` trait polarity. This becomes even weirder when considering that we (or at least, I) want to have `async` trait bounds expressed like:

```
where T: for<'a> async Fn(&'a ()) -> i32,
```

and not:

```
where T: async for<'a> Fn(&'a ()) -> i32,
```

### Fallout

No crates on crater use this syntax, presumably because it's literally useless. This will require modifying the reference grammar, though.

### Alternatives

If this is not desirable, then we can alternatively keep parsing `for<'a>` after the `?` but deprecate it with either an FCW (or an immediate hard error), and begin parsing `for<'a>` *before* the `?`.
This commit is contained in:
Matthias Krüger 2024-07-25 04:43:18 +02:00 committed by GitHub
commit cfc5f25b3d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 201 additions and 68 deletions

View file

@ -935,9 +935,14 @@ impl<'a> Parser<'a> {
/// If no modifiers are present, this does not consume any tokens.
///
/// ```ebnf
/// TRAIT_BOUND_MODIFIERS = [["~"] "const"] ["async"] ["?" | "!"]
/// CONSTNESS = [["~"] "const"]
/// ASYNCNESS = ["async"]
/// POLARITY = ["?" | "!"]
/// ```
///
/// See `parse_generic_ty_bound` for the complete grammar of trait bound modifiers.
fn parse_trait_bound_modifiers(&mut self) -> PResult<'a, TraitBoundModifiers> {
let modifier_lo = self.token.span;
let constness = if self.eat(&token::Tilde) {
let tilde = self.prev_token.span;
self.expect_keyword(kw::Const)?;
@ -970,6 +975,7 @@ impl<'a> Parser<'a> {
} else {
BoundAsyncness::Normal
};
let modifier_hi = self.prev_token.span;
let polarity = if self.eat(&token::Question) {
BoundPolarity::Maybe(self.prev_token.span)
@ -980,13 +986,40 @@ impl<'a> Parser<'a> {
BoundPolarity::Positive
};
// Enforce the mutual-exclusivity of `const`/`async` and `?`/`!`.
match polarity {
BoundPolarity::Positive => {
// All trait bound modifiers allowed to combine with positive polarity
}
BoundPolarity::Maybe(polarity_span) | BoundPolarity::Negative(polarity_span) => {
match (asyncness, constness) {
(BoundAsyncness::Normal, BoundConstness::Never) => {
// Ok, no modifiers.
}
(_, _) => {
let constness = constness.as_str();
let asyncness = asyncness.as_str();
let glue =
if !constness.is_empty() && !asyncness.is_empty() { " " } else { "" };
let modifiers_concatenated = format!("{constness}{glue}{asyncness}");
self.dcx().emit_err(errors::PolarityAndModifiers {
polarity_span,
polarity: polarity.as_str(),
modifiers_span: modifier_lo.to(modifier_hi),
modifiers_concatenated,
});
}
}
}
}
Ok(TraitBoundModifiers { constness, asyncness, polarity })
}
/// Parses a type bound according to:
/// ```ebnf
/// TY_BOUND = TY_BOUND_NOPAREN | (TY_BOUND_NOPAREN)
/// TY_BOUND_NOPAREN = [TRAIT_BOUND_MODIFIERS] [for<LT_PARAM_DEFS>] SIMPLE_PATH
/// TY_BOUND_NOPAREN = [for<GENERIC_PARAMS> CONSTNESS ASYNCNESS | POLARITY] SIMPLE_PATH
/// ```
///
/// For example, this grammar accepts `for<'a: 'b> ~const ?m::Trait<'a>`.
@ -996,9 +1029,25 @@ impl<'a> Parser<'a> {
has_parens: bool,
leading_token: &Token,
) -> PResult<'a, GenericBound> {
let modifiers = self.parse_trait_bound_modifiers()?;
let (mut lifetime_defs, binder_span) = self.parse_late_bound_lifetime_defs()?;
let modifiers_lo = self.token.span;
let modifiers = self.parse_trait_bound_modifiers()?;
let modifiers_span = modifiers_lo.to(self.prev_token.span);
if let Some(binder_span) = binder_span {
match modifiers.polarity {
BoundPolarity::Negative(polarity_span) | BoundPolarity::Maybe(polarity_span) => {
self.dcx().emit_err(errors::BinderAndPolarity {
binder_span,
polarity_span,
polarity: modifiers.polarity.as_str(),
});
}
BoundPolarity::Positive => {}
}
}
// Recover erroneous lifetime bound with modifiers or binder.
// e.g. `T: for<'a> 'a` or `T: ~const 'a`.
if self.token.is_lifetime() {
@ -1006,6 +1055,11 @@ impl<'a> Parser<'a> {
return self.parse_generic_lt_bound(lo, has_parens);
}
if let (more_lifetime_defs, Some(binder_span)) = self.parse_late_bound_lifetime_defs()? {
lifetime_defs.extend(more_lifetime_defs);
self.dcx().emit_err(errors::BinderBeforeModifiers { binder_span, modifiers_span });
}
let mut path = if self.token.is_keyword(kw::Fn)
&& self.look_ahead(1, |tok| tok.kind == TokenKind::OpenDelim(Delimiter::Parenthesis))
&& let Some(path) = self.recover_path_from_fn()