Rollup merge of rust-lang#145474 - fmease:paren-use-bounds-fix, r=fee1-dead

Properly recover from parenthesized use-bounds (precise capturing lists) plus small cleanups

Fixes rust-lang#145470.

First commit fixes the issue, second one performs some desperately needed cleanups.

The fix shouldn't be a breaking change because IINM the parser always ensures that all brackets are balanced (via a buffer of brackets). Meaning even though we used to accept `(use<>` as a valid precise capturing list, it was guaranteed that we would fail in the end.

Author: jhpratt

compiler/rustc_parse/messages.ftl

@@ -748,9 +748,6 @@ parse_parentheses_with_struct_fields = invalid `struct` delimiters or `fn` call
     .suggestion_braces_for_struct = if `{$type}` is a struct, use braces as delimiters
     .suggestion_no_fields_for_fn = if `{$type}` is a function, use the arguments directly
 
-parse_parenthesized_lifetime = parenthesized lifetime bounds are not supported
-parse_parenthesized_lifetime_suggestion = remove the parentheses
-
 parse_path_double_colon = path separator must be a double colon
     .suggestion = use a double colon instead
 

compiler/rustc_parse/src/errors.rs

@@ -3163,27 +3163,6 @@ pub(crate) struct ModifierLifetime {
     pub modifier: &'static str,
 }
 
-#[derive(Subdiagnostic)]
-#[multipart_suggestion(
-    parse_parenthesized_lifetime_suggestion,
-    applicability = "machine-applicable"
-)]
-pub(crate) struct RemoveParens {
-    #[suggestion_part(code = "")]
-    pub lo: Span,
-    #[suggestion_part(code = "")]
-    pub hi: Span,
-}
-
-#[derive(Diagnostic)]
-#[diag(parse_parenthesized_lifetime)]
-pub(crate) struct ParenthesizedLifetime {
-    #[primary_span]
-    pub span: Span,
-    #[subdiagnostic]
-    pub sugg: RemoveParens,
-}
-
 #[derive(Diagnostic)]
 #[diag(parse_underscore_literal_suffix)]
 pub(crate) struct UnderscoreLiteralSuffix {

compiler/rustc_parse/src/parser/expr.rs

@@ -2397,12 +2397,12 @@ impl<'a> Parser<'a> {
         let before = self.prev_token;
         let binder = if self.check_keyword(exp!(For)) {
             let lo = self.token.span;
-            let (lifetime_defs, _) = self.parse_late_bound_lifetime_defs()?;
+            let (bound_vars, _) = self.parse_higher_ranked_binder()?;
             let span = lo.to(self.prev_token.span);
 
             self.psess.gated_spans.gate(sym::closure_lifetime_binder, span);
 
-            ClosureBinder::For { span, generic_params: lifetime_defs }
+            ClosureBinder::For { span, generic_params: bound_vars }
         } else {
             ClosureBinder::NotPresent
         };

compiler/rustc_parse/src/parser/generics.rs

@@ -172,8 +172,11 @@ impl<'a> Parser<'a> {
         })
     }
 
-    /// Parses a (possibly empty) list of lifetime and type parameters, possibly including
-    /// a trailing comma and erroneous trailing attributes.
+    /// Parse a (possibly empty) list of generic (lifetime, type, const) parameters.
+    ///
+    /// ```ebnf
+    /// GenericParams = (GenericParam ("," GenericParam)* ","?)?
+    /// ```
     pub(super) fn parse_generic_params(&mut self) -> PResult<'a, ThinVec<ast::GenericParam>> {
         let mut params = ThinVec::new();
         let mut done = false;
@@ -520,15 +523,15 @@ 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 (bound_vars, _) = self.parse_higher_ranked_binder()?;
 
         // Parse type with mandatory colon and (possibly empty) bounds,
         // or with mandatory equality sign and the second type.
         let ty = self.parse_ty_for_where_clause()?;
         if self.eat(exp!(Colon)) {
             let bounds = self.parse_generic_bounds()?;
             Ok(ast::WherePredicateKind::BoundPredicate(ast::WhereBoundPredicate {
-                bound_generic_params: lifetime_defs,
+                bound_generic_params: bound_vars,
                 bounded_ty: ty,
                 bounds,
             }))

compiler/rustc_parse/src/parser/ty.rs

@@ -308,11 +308,11 @@ 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 (bound_vars, _) = self.parse_higher_ranked_binder()?;
             if self.check_fn_front_matter(false, Case::Sensitive) {
                 self.parse_ty_fn_ptr(
                     lo,
-                    lifetime_defs,
+                    bound_vars,
                     Some(self.prev_token.span.shrink_to_lo()),
                     recover_return_sign,
                 )?
@@ -326,7 +326,7 @@ impl<'a> Parser<'a> {
                     let path = self.parse_path(PathStyle::Type)?;
                     let parse_plus = allow_plus == AllowPlus::Yes && self.check_plus();
                     let kind = self.parse_remaining_bounds_path(
-                        lifetime_defs,
+                        bound_vars,
                         path,
                         lo,
                         parse_plus,
@@ -359,7 +359,7 @@ impl<'a> Parser<'a> {
                     let path = self.parse_path(PathStyle::Type)?;
                     let parse_plus = allow_plus == AllowPlus::Yes && self.check_plus();
                     self.parse_remaining_bounds_path(
-                        lifetime_defs,
+                        bound_vars,
                         path,
                         lo,
                         parse_plus,
@@ -443,7 +443,7 @@ impl<'a> Parser<'a> {
             let ty = ts.into_iter().next().unwrap();
             let maybe_bounds = allow_plus == AllowPlus::Yes && self.token.is_like_plus();
             match ty.kind {
-                // `(TY_BOUND_NOPAREN) + BOUND + ...`.
+                // `"(" BareTraitBound ")" "+" Bound "+" ...`.
                 TyKind::Path(None, path) if maybe_bounds => self.parse_remaining_bounds_path(
                     ThinVec::new(),
                     path,
@@ -853,10 +853,13 @@ impl<'a> Parser<'a> {
         Ok(TyKind::ImplTrait(ast::DUMMY_NODE_ID, bounds))
     }
 
-    fn parse_precise_capturing_args(
-        &mut self,
-    ) -> PResult<'a, (ThinVec<PreciseCapturingArg>, Span)> {
-        let lo = self.token.span;
+    /// Parse a use-bound aka precise capturing list.
+    ///
+    /// ```ebnf
+    /// UseBound = "use" "<" (PreciseCapture ("," PreciseCapture)* ","?)? ">"
+    /// PreciseCapture = "Self" | Ident | Lifetime
+    /// ```
+    fn parse_use_bound(&mut self, lo: Span, parens: ast::Parens) -> PResult<'a, GenericBound> {
         self.expect_lt()?;
         let (args, _, _) = self.parse_seq_to_before_tokens(
             &[exp!(Gt)],
@@ -882,7 +885,13 @@ impl<'a> Parser<'a> {
             },
         )?;
         self.expect_gt()?;
-        Ok((args, lo.to(self.prev_token.span)))
+
+        if let ast::Parens::Yes = parens {
+            self.expect(exp!(CloseParen))?;
+            self.report_parenthesized_bound(lo, self.prev_token.span, "precise capturing lists");
+        }
+
+        Ok(GenericBound::Use(args, lo.to(self.prev_token.span)))
     }
 
     /// Is a `dyn B0 + ... + Bn` type allowed here?
@@ -940,9 +949,10 @@ impl<'a> Parser<'a> {
         self.parse_generic_bounds_common(AllowPlus::Yes)
     }
 
-    /// Parses bounds of a type parameter `BOUND + BOUND + ...`, possibly with trailing `+`.
+    /// Parse generic bounds.
     ///
-    /// See `parse_generic_bound` for the `BOUND` grammar.
+    /// Only if `allow_plus` this parses a `+`-separated list of bounds (trailing `+` is admitted).
+    /// Otherwise, this only parses a single bound or none.
     fn parse_generic_bounds_common(&mut self, allow_plus: AllowPlus) -> PResult<'a, GenericBounds> {
         let mut bounds = Vec::new();
 
@@ -988,48 +998,56 @@ impl<'a> Parser<'a> {
             || self.check_keyword(exp!(Use))
     }
 
-    /// Parses a bound according to the grammar:
+    /// Parse a bound.
+    ///
     /// ```ebnf
-    /// BOUND = TY_BOUND | LT_BOUND
+    /// Bound = LifetimeBound | UseBound | TraitBound
     /// ```
     fn parse_generic_bound(&mut self) -> PResult<'a, GenericBound> {
-        let lo = self.token.span;
         let leading_token = self.prev_token;
+        let lo = self.token.span;
+
+        // We only admit parenthesized *trait* bounds. However, we want to gracefully recover from
+        // other kinds of parenthesized bounds, so parse the opening parenthesis *here*.
+        //
+        // In the future we might want to lift this syntactic restriction and
+        // introduce "`GenericBound::Paren(Box<GenericBound>)`".
         let parens = if self.eat(exp!(OpenParen)) { ast::Parens::Yes } else { ast::Parens::No };
 
-        let bound = if self.token.is_lifetime() {
-            self.parse_generic_lt_bound(lo, parens)?
+        if self.token.is_lifetime() {
+            self.parse_lifetime_bound(lo, parens)
         } else if self.eat_keyword(exp!(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
-            // for order, duplication, and whether they actually reference params.
-            let use_span = self.prev_token.span;
-            let (args, args_span) = self.parse_precise_capturing_args()?;
-            GenericBound::Use(args, use_span.to(args_span))
+            self.parse_use_bound(lo, parens)
         } else {
-            self.parse_generic_ty_bound(lo, parens, &leading_token)?
-        };
-
-        Ok(bound)
+            self.parse_trait_bound(lo, parens, &leading_token)
+        }
     }
 
-    /// Parses a lifetime ("outlives") bound, e.g. `'a`, according to:
+    /// Parse a lifetime-bound aka outlives-bound.
+    ///
     /// ```ebnf
-    /// LT_BOUND = LIFETIME
+    /// LifetimeBound = Lifetime
     /// ```
-    fn parse_generic_lt_bound(
-        &mut self,
-        lo: Span,
-        parens: ast::Parens,
-    ) -> PResult<'a, GenericBound> {
+    fn parse_lifetime_bound(&mut self, lo: Span, parens: ast::Parens) -> PResult<'a, GenericBound> {
         let lt = self.expect_lifetime();
-        let bound = GenericBound::Outlives(lt);
+
         if let ast::Parens::Yes = parens {
-            // FIXME(Centril): Consider not erroring here and accepting `('lt)` instead,
-            // possibly introducing `GenericBound::Paren(Box<GenericBound>)`?
-            self.recover_paren_lifetime(lo)?;
+            self.expect(exp!(CloseParen))?;
+            self.report_parenthesized_bound(lo, self.prev_token.span, "lifetime bounds");
         }
-        Ok(bound)
+
+        Ok(GenericBound::Outlives(lt))
+    }
+
+    fn report_parenthesized_bound(&self, lo: Span, hi: Span, kind: &str) -> ErrorGuaranteed {
+        let mut diag =
+            self.dcx().struct_span_err(lo.to(hi), format!("{kind} may not be parenthesized"));
+        diag.multipart_suggestion(
+            "remove the parentheses",
+            vec![(lo, String::new()), (hi, String::new())],
+            Applicability::MachineApplicable,
+        );
+        diag.emit()
     }
 
     /// Emits an error if any trait bound modifiers were present.
@@ -1074,27 +1092,17 @@ impl<'a> Parser<'a> {
         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) -> PResult<'a, ()> {
-        self.expect(exp!(CloseParen))?;
-        let span = lo.to(self.prev_token.span);
-        let sugg = errors::RemoveParens { lo, hi: self.prev_token.span };
-
-        self.dcx().emit_err(errors::ParenthesizedLifetime { span, sugg });
-        Ok(())
-    }
-
     /// Parses the modifiers that may precede a trait in a bound, e.g. `?Trait` or `[const] Trait`.
     ///
     /// If no modifiers are present, this does not consume any tokens.
     ///
     /// ```ebnf
-    /// CONSTNESS = [["["] "const" ["]"]]
-    /// ASYNCNESS = ["async"]
-    /// POLARITY = ["?" | "!"]
+    /// Constness = ("const" | "[" "const" "]")?
+    /// Asyncness = "async"?
+    /// Polarity = ("?" | "!")?
     /// ```
     ///
-    /// See `parse_generic_ty_bound` for the complete grammar of trait bound modifiers.
+    /// See `parse_trait_bound` for more context.
     fn parse_trait_bound_modifiers(&mut self) -> PResult<'a, TraitBoundModifiers> {
         let modifier_lo = self.token.span;
         let constness = self.parse_bound_constness()?;
@@ -1187,20 +1195,21 @@ impl<'a> Parser<'a> {
         })
     }
 
-    /// Parses a type bound according to:
+    /// Parse a trait bound.
+    ///
     /// ```ebnf
-    /// TY_BOUND = TY_BOUND_NOPAREN | (TY_BOUND_NOPAREN)
-    /// TY_BOUND_NOPAREN = [for<GENERIC_PARAMS> CONSTNESS ASYNCNESS | POLARITY] SIMPLE_PATH
+    /// TraitBound = BareTraitBound | "(" BareTraitBound ")"
+    /// BareTraitBound =
+    ///     (HigherRankedBinder Constness Asyncness | Polarity)
+    ///     TypePath
     /// ```
-    ///
-    /// For example, this grammar accepts `for<'a: 'b> [const] ?m::Trait<'a>`.
-    fn parse_generic_ty_bound(
+    fn parse_trait_bound(
         &mut self,
         lo: Span,
         parens: ast::Parens,
         leading_token: &Token,
     ) -> PResult<'a, GenericBound> {
-        let (mut lifetime_defs, binder_span) = self.parse_late_bound_lifetime_defs()?;
+        let (mut bound_vars, binder_span) = self.parse_higher_ranked_binder()?;
 
         let modifiers_lo = self.token.span;
         let modifiers = self.parse_trait_bound_modifiers()?;
@@ -1223,11 +1232,11 @@ impl<'a> Parser<'a> {
         // 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, parens);
+            return self.parse_lifetime_bound(lo, parens);
         }
 
-        if let (more_lifetime_defs, Some(binder_span)) = self.parse_late_bound_lifetime_defs()? {
-            lifetime_defs.extend(more_lifetime_defs);
+        if let (more_bound_vars, Some(binder_span)) = self.parse_higher_ranked_binder()? {
+            bound_vars.extend(more_bound_vars);
             self.dcx().emit_err(errors::BinderBeforeModifiers { binder_span, modifiers_span });
         }
 
@@ -1287,7 +1296,7 @@ impl<'a> Parser<'a> {
         };
 
         if self.may_recover() && self.token == TokenKind::OpenParen {
-            self.recover_fn_trait_with_lifetime_params(&mut path, &mut lifetime_defs)?;
+            self.recover_fn_trait_with_lifetime_params(&mut path, &mut bound_vars)?;
         }
 
         if let ast::Parens::Yes = parens {
@@ -1310,7 +1319,7 @@ impl<'a> Parser<'a> {
         }
 
         let poly_trait =
-            PolyTraitRef::new(lifetime_defs, path, modifiers, lo.to(self.prev_token.span), parens);
+            PolyTraitRef::new(bound_vars, path, modifiers, lo.to(self.prev_token.span), parens);
         Ok(GenericBound::Trait(poly_trait))
     }
 
@@ -1349,8 +1358,12 @@ impl<'a> Parser<'a> {
         }
     }
 
-    /// Optionally parses `for<$generic_params>`.
-    pub(super) fn parse_late_bound_lifetime_defs(
+    /// Parse an optional higher-ranked binder.
+    ///
+    /// ```ebnf
+    /// HigherRankedBinder = ("for" "<" GenericParams ">")?
+    /// ```
+    pub(super) fn parse_higher_ranked_binder(
         &mut self,
     ) -> PResult<'a, (ThinVec<GenericParam>, Option<Span>)> {
         if self.eat_keyword(exp!(For)) {

tests/ui/impl-trait/precise-capturing/parenthesized.rs

@@ -0,0 +1,8 @@
+// Ensure that we forbid parenthesized use-bounds. In the future we might want
+// to lift this restriction but for now they bear no use whatsoever.
+
+fn f() -> impl Sized + (use<>) {}
+//~^ ERROR precise capturing lists may not be parenthesized
+//~| HELP remove the parentheses
+
+fn main() {}

tests/ui/impl-trait/precise-capturing/parenthesized.stderr

@@ -0,0 +1,14 @@
+error: precise capturing lists may not be parenthesized
+  --> $DIR/parenthesized.rs:4:24
+   |
+LL | fn f() -> impl Sized + (use<>) {}
+   |                        ^^^^^^^
+   |
+help: remove the parentheses
+   |
+LL - fn f() -> impl Sized + (use<>) {}
+LL + fn f() -> impl Sized + use<> {}
+   |
+
+error: aborting due to 1 previous error
+