Auto merge of #127054 - compiler-errors:bound-ordering, r=<try>

Reorder trait bound modifiers *after* `for<...>` binder in trait bounds r? `@fmease` or reassign if you're busy I wanted to crater this to see what the consequences are. Otherwise, we may be able to make this into a lint if the crater fallout is really bad. The motivation for this is because we currently parse: ```rust T: async for<'a> Fn(&'a ()) ``` Instead of: ```rust T: for<'a> async Fn(&'a ()) ``` I believe the latter to be the correct ordering of bounds, since the `async` keyword modifies the *trait* so it should be attached to the trait path.
rust-lang · Jun 27, 2024 · 3a8bed5 · 3a8bed5
2 parents 2495953 + 13bff3f
commit 3a8bed5
Show file tree

Hide file tree

Showing 13 changed files with 102 additions and 51 deletions.
diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl
@@ -61,6 +61,9 @@ parse_bare_cr = {$double_quotes ->
 
 parse_bare_cr_in_raw_string = bare CR not allowed in raw string
 
+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}

diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs
@@ -3028,3 +3028,12 @@ 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,
+}
diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
@@ -2327,7 +2327,7 @@ impl<'a> Parser<'a> {
         let before = self.prev_token.clone();
         let binder = if self.check_keyword(kw::For) {
             let lo = self.token.span;
-            let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
+            let (lifetime_defs, _) = self.parse_late_bound_lifetime_defs()?;
             let span = lo.to(self.prev_token.span);
 
             self.psess.gated_spans.gate(sym::closure_lifetime_binder, span);

diff --git a/compiler/rustc_parse/src/parser/generics.rs b/compiler/rustc_parse/src/parser/generics.rs
@@ -457,7 +457,7 @@ impl<'a> Parser<'a> {
         // * `for<'a> Trait1<'a>: Trait2<'a /* ok */>`
         // * `(for<'a> Trait1<'a>): Trait2<'a /* not ok */>`
         // * `for<'a> for<'b> Trait1<'a, 'b>: Trait2<'a /* ok */, 'b /* not ok */>`
-        let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
+        let (lifetime_defs, _) = self.parse_late_bound_lifetime_defs()?;
 
         // Parse type with mandatory colon and (possibly empty) bounds,
         // or with mandatory equality sign and the second type.

diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs
@@ -18,7 +18,7 @@ use rustc_ast::{
 };
 use rustc_errors::{Applicability, PResult};
 use rustc_span::symbol::{kw, sym, Ident};
-use rustc_span::{Span, Symbol};
+use rustc_span::{ErrorGuaranteed, Span, Symbol};
 use thin_vec::{thin_vec, ThinVec};
 
 #[derive(Copy, Clone, PartialEq)]
@@ -280,7 +280,7 @@ impl<'a> Parser<'a> {
             // Function pointer type or bound list (trait object type) starting with a poly-trait.
             //   `for<'lt> [unsafe] [extern "ABI"] fn (&'lt S) -> T`
             //   `for<'lt> Trait1<'lt> + Trait2 + 'a`
-            let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
+            let (lifetime_defs, _) = self.parse_late_bound_lifetime_defs()?;
             if self.check_fn_front_matter(false, Case::Sensitive) {
                 self.parse_ty_bare_fn(
                     lo,
@@ -833,12 +833,9 @@ impl<'a> Parser<'a> {
         let lo = self.token.span;
         let leading_token = self.prev_token.clone();
         let has_parens = self.eat(&token::OpenDelim(Delimiter::Parenthesis));
-        let inner_lo = self.token.span;
 
-        let modifiers = self.parse_trait_bound_modifiers()?;
         let bound = if self.token.is_lifetime() {
-            self.error_lt_bound_with_modifiers(modifiers);
-            self.parse_generic_lt_bound(lo, inner_lo, has_parens)?
+            self.parse_generic_lt_bound(lo, has_parens)?
         } else if self.eat_keyword(kw::Use) {
             // parse precise captures, if any. This is `use<'lt, 'lt, P, P>`; a list of
             // lifetimes and ident params (including SelfUpper). These are validated later
@@ -848,7 +845,7 @@ impl<'a> Parser<'a> {
             let (args, args_span) = self.parse_precise_capturing_args()?;
             GenericBound::Use(args, use_span.to(args_span))
         } else {
-            self.parse_generic_ty_bound(lo, has_parens, modifiers, &leading_token)?
+            self.parse_generic_ty_bound(lo, has_parens, &leading_token)?
         };
 
         Ok(bound)
@@ -858,50 +855,64 @@ impl<'a> Parser<'a> {
     /// ```ebnf
     /// LT_BOUND = LIFETIME
     /// ```
-    fn parse_generic_lt_bound(
-        &mut self,
-        lo: Span,
-        inner_lo: Span,
-        has_parens: bool,
-    ) -> PResult<'a, GenericBound> {
-        let bound = GenericBound::Outlives(self.expect_lifetime());
+    fn parse_generic_lt_bound(&mut self, lo: Span, has_parens: bool) -> PResult<'a, GenericBound> {
+        let lt = self.expect_lifetime();
+        let bound = GenericBound::Outlives(lt);
         if has_parens {
             // FIXME(Centril): Consider not erroring here and accepting `('lt)` instead,
             // possibly introducing `GenericBound::Paren(P<GenericBound>)`?
-            self.recover_paren_lifetime(lo, inner_lo)?;
+            self.recover_paren_lifetime(lo, lt.ident.span)?;
         }
         Ok(bound)
     }
 
     /// Emits an error if any trait bound modifiers were present.
-    fn error_lt_bound_with_modifiers(&self, modifiers: TraitBoundModifiers) {
-        match modifiers.constness {
+    fn error_lt_bound_with_modifiers(
+        &self,
+        modifiers: TraitBoundModifiers,
+        binder_span: Option<Span>,
+    ) -> ErrorGuaranteed {
+        let TraitBoundModifiers { constness, asyncness, polarity } = modifiers;
+
+        match constness {
             BoundConstness::Never => {}
             BoundConstness::Always(span) | BoundConstness::Maybe(span) => {
-                self.dcx().emit_err(errors::ModifierLifetime {
-                    span,
-                    modifier: modifiers.constness.as_str(),
-                });
+                return self
+                    .dcx()
+                    .emit_err(errors::ModifierLifetime { span, modifier: constness.as_str() });
             }
         }
 
-        match modifiers.polarity {
+        match polarity {
             BoundPolarity::Positive => {}
             BoundPolarity::Negative(span) | BoundPolarity::Maybe(span) => {
-                self.dcx().emit_err(errors::ModifierLifetime {
-                    span,
-                    modifier: modifiers.polarity.as_str(),
-                });
+                return self
+                    .dcx()
+                    .emit_err(errors::ModifierLifetime { span, modifier: polarity.as_str() });
+            }
+        }
+
+        match asyncness {
+            BoundAsyncness::Normal => {}
+            BoundAsyncness::Async(span) => {
+                return self
+                    .dcx()
+                    .emit_err(errors::ModifierLifetime { span, modifier: asyncness.as_str() });
             }
         }
+
+        if let Some(span) = binder_span {
+            return self.dcx().emit_err(errors::ModifierLifetime { span, modifier: "for<...>" });
+        }
+
+        unreachable!("lifetime bound intercepted in `parse_generic_ty_bound` but no modifiers?")
     }
 
     /// Recover on `('lifetime)` with `(` already eaten.
-    fn recover_paren_lifetime(&mut self, lo: Span, inner_lo: Span) -> PResult<'a, ()> {
-        let inner_span = inner_lo.to(self.prev_token.span);
+    fn recover_paren_lifetime(&mut self, lo: Span, lt_span: Span) -> PResult<'a, ()> {
         self.expect(&token::CloseDelim(Delimiter::Parenthesis))?;
         let span = lo.to(self.prev_token.span);
-        let (sugg, snippet) = if let Ok(snippet) = self.span_to_snippet(inner_span) {
+        let (sugg, snippet) = if let Ok(snippet) = self.span_to_snippet(lt_span) {
             (Some(span), snippet)
         } else {
             (None, String::new())
@@ -970,15 +981,31 @@ impl<'a> Parser<'a> {
     /// TY_BOUND_NOPAREN = [TRAIT_BOUND_MODIFIERS] [for<LT_PARAM_DEFS>] SIMPLE_PATH
     /// ```
     ///
-    /// For example, this grammar accepts `~const ?for<'a: 'b> m::Trait<'a>`.
+    /// For example, this grammar accepts `for<'a: 'b> ~const ?m::Trait<'a>`.
     fn parse_generic_ty_bound(
         &mut self,
         lo: Span,
         has_parens: bool,
-        modifiers: TraitBoundModifiers,
         leading_token: &Token,
     ) -> PResult<'a, GenericBound> {
-        let mut lifetime_defs = self.parse_late_bound_lifetime_defs()?;
+        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);
+
+        // 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()
@@ -1094,16 +1121,19 @@ impl<'a> Parser<'a> {
     }
 
     /// Optionally parses `for<$generic_params>`.
-    pub(super) fn parse_late_bound_lifetime_defs(&mut self) -> PResult<'a, ThinVec<GenericParam>> {
+    pub(super) fn parse_late_bound_lifetime_defs(
+        &mut self,
+    ) -> PResult<'a, (ThinVec<GenericParam>, Option<Span>)> {
         if self.eat_keyword(kw::For) {
+            let lo = self.token.span;
             self.expect_lt()?;
             let params = self.parse_generic_params()?;
             self.expect_gt()?;
-            // We rely on AST validation to rule out invalid cases: There must not be type
-            // parameters, and the lifetime parameters must not have bounds.
-            Ok(params)
+            // We rely on AST validation to rule out invalid cases: There must not be
+            // type or const parameters, and parameters must not have bounds.
+            Ok((params, Some(lo.to(self.prev_token.span))))
         } else {
-            Ok(ThinVec::new())
+            Ok((ThinVec::new(), None))
         }
     }
 

diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs
@@ -16,7 +16,7 @@ use std::path::{Path, PathBuf};
 const ENTRY_LIMIT: u32 = 900;
 // 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

diff --git a/tests/ui/async-await/async-fn/higher-ranked-async-fn.rs b/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;

diff --git a/tests/ui/impl-trait/normalize-tait-in-const.rs b/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());
 }
 

diff --git a/tests/ui/impl-trait/normalize-tait-in-const.stderr b/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 | }

diff --git a/tests/ui/issues/issue-39089.rs b/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() {}
diff --git a/tests/ui/issues/issue-39089.stderr b/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
+
diff --git a/tests/ui/parser/bounds-type.rs b/tests/ui/parser/bounds-type.rs
@@ -5,7 +5,7 @@ struct S<
     T: Tr + 'a, // OK
     T: 'a, // OK
     T:, // OK
-    T: ?for<'a> Trait, // OK
+    T: for<'a> ?Trait, // OK
     T: Tr +, // OK
     T: ?'a, //~ ERROR `?` may only modify trait bounds, not lifetime bounds
 

diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/tilde-const-syntax.rs b/tests/ui/rfcs/rfc-2632-const-trait-impl/tilde-const-syntax.rs
@@ -4,6 +4,6 @@
 #![feature(const_trait_impl)]
 
 struct S<
-    T: ~const ?for<'a> Tr<'a> + 'static + ~const std::ops::Add,
-    T: ~const ?for<'a: 'b> m::Trait<'a>,
+    T: for<'a> ~const ?Tr<'a> + 'static + ~const std::ops::Add,
+    T: for<'a: 'b> ~const ?m::Trait<'a>,
 >;