Rollup merge of rust-lang#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 `?`.

Author: matthiaskrgr

compiler/rustc_ast_passes/messages.ftl

@@ -155,8 +155,6 @@ ast_passes_impl_trait_path = `impl Trait` is not allowed in path parameters
 ast_passes_incompatible_features = `{$f1}` and `{$f2}` are incompatible, using them at the same time is not allowed
     .help = remove one of these features
 
-ast_passes_incompatible_trait_bound_modifiers = `{$left}` and `{$right}` are mutually exclusive
-
 ast_passes_inherent_cannot_be = inherent impls cannot be {$annotation}
     .because = {$annotation} because of this
     .type = inherent impl for this type

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -1366,17 +1366,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     {
                         self.dcx().emit_err(errors::TildeConstDisallowed { span, reason });
                     }
-                    (
-                        _,
-                        BoundConstness::Always(_) | BoundConstness::Maybe(_),
-                        BoundPolarity::Negative(_) | BoundPolarity::Maybe(_),
-                    ) => {
-                        self.dcx().emit_err(errors::IncompatibleTraitBoundModifiers {
-                            span: bound.span(),
-                            left: modifiers.constness.as_str(),
-                            right: modifiers.polarity.as_str(),
-                        });
-                    }
                     _ => {}
                 }
 

compiler/rustc_ast_passes/src/errors.rs

@@ -656,15 +656,6 @@ pub enum TildeConstReason {
     Item,
 }
 
-#[derive(Diagnostic)]
-#[diag(ast_passes_incompatible_trait_bound_modifiers)]
-pub struct IncompatibleTraitBoundModifiers {
-    #[primary_span]
-    pub span: Span,
-    pub left: &'static str,
-    pub right: &'static str,
-}
-
 #[derive(Diagnostic)]
 #[diag(ast_passes_const_and_async)]
 pub struct ConstAndAsync {

compiler/rustc_parse/messages.ftl

@@ -53,6 +53,12 @@ parse_bare_cr = {$double_quotes ->
 
 parse_bare_cr_in_raw_string = bare CR not allowed in raw string
 
+parse_binder_and_polarity = `for<...>` binder not allowed with `{$polarity}` trait polarity modifier
+    .label = there is not a well-defined meaning for a higher-ranked `{$polarity}` trait
+
+parse_binder_before_modifiers = `for<...>` binder should be placed before trait bound modifiers
+    .label = place the `for<...>` binder before any modifiers
+
 parse_bounds_not_allowed_on_trait_aliases = bounds are not allowed on trait aliases
 
 parse_box_not_pat = expected pattern, found {$descr}
@@ -577,6 +583,9 @@ parse_missing_trait_in_trait_impl = missing trait in a trait impl
 parse_modifier_lifetime = `{$modifier}` may only modify trait bounds, not lifetime bounds
     .suggestion = remove the `{$modifier}`
 
+parse_modifiers_and_polarity = `{$modifiers_concatenated}` trait not allowed with `{$polarity}` trait polarity modifier
+    .label = there is not a well-defined meaning for a `{$modifiers_concatenated} {$polarity}` trait
+
 parse_more_than_one_char = character literal may only contain one codepoint
     .followed_by = this `{$chr}` is followed by the combining {$len ->
         [one] mark

compiler/rustc_parse/src/errors.rs

@@ -3212,3 +3212,33 @@ pub struct UnsafeAttrOutsideUnsafeSuggestion {
     #[suggestion_part(code = ")")]
     pub right: Span,
 }
+
+#[derive(Diagnostic)]
+#[diag(parse_binder_before_modifiers)]
+pub struct BinderBeforeModifiers {
+    #[primary_span]
+    pub binder_span: Span,
+    #[label]
+    pub modifiers_span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(parse_binder_and_polarity)]
+pub struct BinderAndPolarity {
+    #[primary_span]
+    pub polarity_span: Span,
+    #[label]
+    pub binder_span: Span,
+    pub polarity: &'static str,
+}
+
+#[derive(Diagnostic)]
+#[diag(parse_modifiers_and_polarity)]
+pub struct PolarityAndModifiers {
+    #[primary_span]
+    pub polarity_span: Span,
+    #[label]
+    pub modifiers_span: Span,
+    pub polarity: &'static str,
+    pub modifiers_concatenated: String,
+}

compiler/rustc_parse/src/parser/ty.rs

@@ -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,16 +1029,37 @@ 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() {
             let _: ErrorGuaranteed = self.error_lt_bound_with_modifiers(modifiers, binder_span);
             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()

src/tools/rustfmt/tests/source/type.rs

@@ -146,8 +146,6 @@ trait T: ~   const  Super {}
 
 const fn not_quite_const<S: ~  const    T>() -> i32 { <S as T>::CONST }
 
-struct S<T:~  const   ?  Sized>(std::marker::PhantomData<T>);
-
 impl ~    const T {}
 
 fn apit(_: impl ~   const T) {}

src/tools/rustfmt/tests/target/negative-bounds.rs

@@ -3,9 +3,3 @@ where
     i32: !Copy,
 {
 }
-
-fn maybe_const_negative()
-where
-    i32: ~const !Copy,
-{
-}

src/tools/rustfmt/tests/target/type.rs

@@ -153,8 +153,6 @@ const fn not_quite_const<S: ~const T>() -> i32 {
     <S as T>::CONST
 }
 
-struct S<T: ~const ?Sized>(std::marker::PhantomData<T>);
-
 impl ~const T {}
 
 fn apit(_: impl ~const T) {}

src/tools/tidy/src/ui_tests.rs

@@ -16,7 +16,7 @@ use std::path::{Path, PathBuf};
 const ENTRY_LIMIT: u32 = 901;
 // FIXME: The following limits should be reduced eventually.
 
-const ISSUES_ENTRY_LIMIT: u32 = 1672;
+const ISSUES_ENTRY_LIMIT: u32 = 1673;
 
 const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
     "rs",     // test source files

tests/ui/async-await/async-fn/higher-ranked-async-fn.rs

@@ -15,7 +15,7 @@ async fn f(arg: &i32) {}
 
 async fn func<F>(f: F)
 where
-    F: async for<'a> Fn(&'a i32),
+    F: for<'a> async Fn(&'a i32),
 {
     let x: i32 = 0;
     f(&x).await;

tests/ui/impl-trait/normalize-tait-in-const.rs

@@ -24,7 +24,7 @@ mod foo {
 }
 use foo::*;
 
-const fn with_positive<F: ~const for<'a> Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
+const fn with_positive<F: for<'a> ~const Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
     fun(filter_positive());
 }
 

tests/ui/impl-trait/normalize-tait-in-const.stderr

@@ -1,13 +1,13 @@
 error: `~const` can only be applied to `#[const_trait]` traits
   --> $DIR/normalize-tait-in-const.rs:27:42
    |
-LL | const fn with_positive<F: ~const for<'a> Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
+LL | const fn with_positive<F: for<'a> ~const Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
    |                                          ^^^^^^^^^^^^^^^^^
 
 error: `~const` can only be applied to `#[const_trait]` traits
   --> $DIR/normalize-tait-in-const.rs:27:69
    |
-LL | const fn with_positive<F: ~const for<'a> Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
+LL | const fn with_positive<F: for<'a> ~const Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
    |                                                                     ^^^^^^^^
 
 error[E0015]: cannot call non-const closure in constant functions
@@ -19,7 +19,7 @@ LL |     fun(filter_positive());
    = note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
 help: consider further restricting this bound
    |
-LL | const fn with_positive<F: ~const for<'a> Fn(&'a Alias<'a>) + ~const Destruct + ~const Fn(&foo::Alias<'_>)>(fun: F) {
+LL | const fn with_positive<F: for<'a> ~const Fn(&'a Alias<'a>) + ~const Destruct + ~const Fn(&foo::Alias<'_>)>(fun: F) {
    |                                                                              ++++++++++++++++++++++++++++
 help: add `#![feature(effects)]` to the crate attributes to enable
    |
@@ -29,7 +29,7 @@ LL + #![feature(effects)]
 error[E0493]: destructor of `F` cannot be evaluated at compile-time
   --> $DIR/normalize-tait-in-const.rs:27:79
    |
-LL | const fn with_positive<F: ~const for<'a> Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
+LL | const fn with_positive<F: for<'a> ~const Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
    |                                                                               ^^^ the destructor for this type cannot be evaluated in constant functions
 LL |     fun(filter_positive());
 LL | }

tests/ui/issues/issue-39089.rs

@@ -1,5 +1,4 @@
-//@ check-pass
-#![allow(dead_code)]
 fn f<T: ?for<'a> Sized>() {}
+//~^ ERROR `for<...>` binder should be placed before trait bound modifiers
 
 fn main() {}

tests/ui/issues/issue-39089.stderr

@@ -0,0 +1,10 @@
+error: `for<...>` binder should be placed before trait bound modifiers
+  --> $DIR/issue-39089.rs:1:13
+   |
+LL | fn f<T: ?for<'a> Sized>() {}
+   |         -   ^^^^
+   |         |
+   |         place the `for<...>` binder before any modifiers
+
+error: aborting due to 1 previous error
+

tests/ui/parser/bounds-type.rs

@@ -1,19 +1,30 @@
 //@ compile-flags: -Z parse-only
+//@ edition: 2021
 
 struct S<
     T: 'a + Tr, // OK
     T: Tr + 'a, // OK
     T: 'a, // OK
     T:, // OK
-    T: ?for<'a> Trait, // OK
+    T: for<'a> ?Trait, //~ ERROR `for<...>` binder not allowed with `?` trait polarity modifier
     T: Tr +, // OK
     T: ?'a, //~ ERROR `?` may only modify trait bounds, not lifetime bounds
 
     T: ~const Tr, // OK
-    T: ~const ?Tr, // OK
+    T: ~const ?Tr, //~ ERROR `~const` trait not allowed with `?` trait polarity modifier
     T: ~const Tr + 'a, // OK
     T: ~const 'a, //~ ERROR `~const` may only modify trait bounds, not lifetime bounds
     T: const 'a, //~ ERROR `const` may only modify trait bounds, not lifetime bounds
+
+    T: async Tr, // OK
+    T: async ?Tr, //~ ERROR `async` trait not allowed with `?` trait polarity modifier
+    T: async Tr + 'a, // OK
+    T: async 'a, //~ ERROR `async` may only modify trait bounds, not lifetime bounds
+
+    T: const async Tr, // OK
+    T: const async ?Tr, //~ ERROR `const async` trait not allowed with `?` trait polarity modifier
+    T: const async Tr + 'a, // OK
+    T: const async 'a, //~ ERROR `const` may only modify trait bounds, not lifetime bounds
 >;
 
 fn main() {}