Recover from some const expr parse errors

Author: estebank

src/librustc_ast/ast.rs

@@ -228,6 +228,15 @@ pub enum AngleBracketedArg {
     Constraint(AssocTyConstraint),
 }
 
+impl AngleBracketedArg {
+    pub fn span(&self) -> Span {
+        match self {
+            AngleBracketedArg::Arg(arg) => arg.span(),
+            AngleBracketedArg::Constraint(constraint) => constraint.span,
+        }
+    }
+}
+
 impl Into<Option<P<GenericArgs>>> for AngleBracketedArgs {
     fn into(self) -> Option<P<GenericArgs>> {
         Some(P(GenericArgs::AngleBracketed(self)))

src/librustc_ast/token.rs

@@ -313,6 +313,13 @@ impl TokenKind {
             _ => None,
         }
     }
+
+    pub fn should_end_const_arg(&self) -> bool {
+        match self {
+            Gt | Ge | BinOp(Shr) | BinOpEq(Shr) => true,
+            _ => false,
+        }
+    }
 }
 
 impl Token {

src/librustc_parse/parser/expr.rs

@@ -329,12 +329,18 @@ impl<'a> Parser<'a> {
     /// The method does not advance the current token.
     ///
     /// Also performs recovery for `and` / `or` which are mistaken for `&&` and `||` respectively.
-    fn check_assoc_op(&self) -> Option<Spanned<AssocOp>> {
+    crate fn check_assoc_op(&self) -> Option<Spanned<AssocOp>> {
         let (op, span) = match (AssocOp::from_token(&self.token), self.token.ident()) {
-            // When parsing const expressions, stop parsing when encountering `<` and `>`.
-            (Some(AssocOp::ShiftRight), _) | (Some(AssocOp::Greater), _)
-                if self.restrictions.contains(Restrictions::CONST_EXPR) =>
-            {
+            // When parsing const expressions, stop parsing when encountering `>`.
+            (
+                Some(
+                    AssocOp::ShiftRight
+                    | AssocOp::Greater
+                    | AssocOp::GreaterEqual
+                    | AssocOp::AssignOp(token::BinOpToken::Shr),
+                ),
+                _,
+            ) if self.restrictions.contains(Restrictions::CONST_EXPR) => {
                 return None;
             }
             (Some(op), _) => (op, self.token.span),

src/librustc_parse/parser/path.rs

@@ -6,7 +6,8 @@ use rustc_ast::ast::{AnonConst, AssocTyConstraint, AssocTyConstraintKind, BlockC
 use rustc_ast::ast::{Ident, Path, PathSegment, QSelf};
 use rustc_ast::ptr::P;
 use rustc_ast::token::{self, Token};
-use rustc_errors::{pluralize, Applicability, PResult};
+use rustc_ast::util::parser::AssocOp;
+use rustc_errors::{pluralize, Applicability, DiagnosticBuilder, PResult};
 use rustc_span::source_map::{BytePos, Span};
 use rustc_span::symbol::{kw, sym};
 
@@ -392,12 +393,110 @@ impl<'a> Parser<'a> {
         while let Some(arg) = self.parse_angle_arg()? {
             args.push(arg);
             if !self.eat(&token::Comma) {
+                if self.token.kind.should_end_const_arg() {
+                    // We will correctly parse a closing `>`, exit.
+                } else {
+                    // Try to recover from possible `const` arg without braces.
+                    let arg = args.pop().unwrap();
+                    // FIXME: for some reason using `unexpected` or `expected_one_of_not_found` has
+                    // adverse side-effects to subsequent errors and seems to advance the parser.
+                    // We are causing this error here exclusively in case that a `const` expression
+                    // could be recovered from the current parser state, even if followed by more
+                    // arguments after a comma.
+                    let mut err = self.struct_span_err(
+                        self.token.span,
+                        &format!(
+                            "expected one of `,` or `>`, found {}",
+                            super::token_descr(&self.token)
+                        ),
+                    );
+                    err.span_label(self.token.span, "expected one of `,` or `>`");
+                    match self.recover_const_arg(arg.span(), err) {
+                        Ok(arg) => {
+                            args.push(AngleBracketedArg::Arg(arg));
+                            if self.eat(&token::Comma) {
+                                continue;
+                            }
+                        }
+                        Err(mut err) => {
+                            args.push(arg);
+                            // We will emit a more generic error later.
+                            err.delay_as_bug();
+                        }
+                    }
+                }
                 break;
             }
         }
         Ok(args)
     }
 
+    /// Try to recover from possible `const` arg without braces.
+    ///
+    /// When encountering code like `foo::< bar + 3 >` or `foo::< bar - baz >` we suggest
+    /// `foo::<{ bar + 3 }>` and `foo::<{ bar - baz }>` respectively. We only provide a suggestion
+    /// when we have a high degree of certainty that the resulting expression would be well formed.
+    pub fn recover_const_arg(
+        &mut self,
+        start: Span,
+        mut err: DiagnosticBuilder<'a>,
+    ) -> PResult<'a, GenericArg> {
+        let is_op = AssocOp::from_token(&self.token)
+            .and_then(|op| {
+                if let AssocOp::Greater
+                | AssocOp::Less
+                | AssocOp::ShiftRight
+                | AssocOp::GreaterEqual
+                | AssocOp::Assign // Don't recover from `foo::<bar = baz>`
+                | AssocOp::AssignOp(_) = op
+                {
+                    None
+                } else {
+                    Some(op)
+                }
+            })
+            .is_some();
+        // This will be true when a trait object type `Foo +` has been parsed.
+        let was_op = self.prev_token.kind == token::BinOp(token::Plus);
+        if !is_op && !was_op {
+            // We perform these checks and early return to avoid taking a snapshot unnecessarily.
+            return Err(err);
+        }
+        let snapshot = self.clone();
+        if is_op {
+            self.bump();
+        }
+        match self.parse_expr_res(Restrictions::CONST_EXPR, None) {
+            Ok(expr) => {
+                if token::Comma == self.token.kind || self.token.kind.should_end_const_arg() {
+                    // Avoid the following output by checking that we consumed a full const arg:
+                    // help: to write a `const` expression, surround it with braces for it to
+                    //       be unambiguous
+                    //    |
+                    // LL |     let sr: Vec<{ (u32, _, _) = vec![] };
+                    //    |                 ^                      ^
+                    err.multipart_suggestion(
+                        "to write a `const` expression, surround it with braces for it to be \
+                        unambiguous",
+                        vec![
+                            (start.shrink_to_lo(), "{ ".to_string()),
+                            (expr.span.shrink_to_hi(), " }".to_string()),
+                        ],
+                        Applicability::MaybeIncorrect,
+                    );
+                    let value = self.mk_expr_err(start.to(expr.span));
+                    err.emit();
+                    return Ok(GenericArg::Const(AnonConst { id: ast::DUMMY_NODE_ID, value }));
+                }
+            }
+            Err(mut err) => {
+                err.cancel();
+            }
+        }
+        *self = snapshot;
+        Err(err)
+    }
+
     /// Parses a single argument in the angle arguments `<...>` of a path segment.
     fn parse_angle_arg(&mut self) -> PResult<'a, Option<AngleBracketedArg>> {
         if self.check_ident() && self.look_ahead(1, |t| matches!(t.kind, token::Eq | token::Colon))
@@ -474,6 +573,7 @@ impl<'a> Parser<'a> {
     /// Parse a generic argument in a path segment.
     /// This does not include constraints, e.g., `Item = u8`, which is handled in `parse_angle_arg`.
     fn parse_generic_arg(&mut self) -> PResult<'a, Option<GenericArg>> {
+        let start = self.token.span;
         let arg = if self.check_lifetime() && self.look_ahead(1, |t| !t.is_like_plus()) {
             // Parse lifetime argument.
             GenericArg::Lifetime(self.expect_lifetime())
@@ -502,7 +602,13 @@ impl<'a> Parser<'a> {
             GenericArg::Const(AnonConst { id: ast::DUMMY_NODE_ID, value })
         } else if self.check_type() {
             // Parse type argument.
-            GenericArg::Type(self.parse_ty()?)
+            match self.parse_ty() {
+                Ok(ty) => GenericArg::Type(ty),
+                Err(err) => {
+                    // Try to recover from possible `const` arg without braces.
+                    return self.recover_const_arg(start, err).map(Some);
+                }
+            }
         } else {
             return Ok(None);
         };

src/test/ui/const-generics/const-expression-missing-braces.rs

@@ -3,19 +3,28 @@
 
 fn foo<const C: usize>() {}
 
+const BAR: usize = 42;
+
 fn a() {
-    let bar = 3;
-    foo::<bar + 3>();
-    //~^ ERROR expected one of `!`, `(`, `,`, `>`, `?`, `for`, lifetime, or path, found `3`
+    foo::<BAR + 3>();
+    //~^ ERROR expected one of
 }
 fn b() {
-    let bar = 3;
-    foo::<bar + bar>();
+    foo::<BAR + BAR>();
     //~^ ERROR likely `const` expression parsed as trait bounds
 }
 fn c() {
-    let bar = 3;
     foo::<3 + 3>(); // ok
 }
-
+fn d() {
+    foo::<BAR - 3>();
+    //~^ ERROR expected one of
+}
+fn e() {
+    foo::<BAR - BAR>();
+    //~^ ERROR expected one of
+}
+fn f() {
+    foo::<100 - BAR>(); // ok
+}
 fn main() {}

src/test/ui/const-generics/const-expression-missing-braces.stderr

@@ -1,19 +1,46 @@
-error: expected one of `!`, `(`, `,`, `>`, `?`, `for`, lifetime, or path, found `3`
-  --> $DIR/const-expression-missing-braces.rs:8:17
+error: expected one of `,` or `>`, found `3`
+  --> $DIR/const-expression-missing-braces.rs:9:17
    |
-LL |     foo::<bar + 3>();
-   |                 ^ expected one of 8 possible tokens
+LL |     foo::<BAR + 3>();
+   |                 ^ expected one of `,` or `>`
+   |
+help: to write a `const` expression, surround it with braces for it to be unambiguous
+   |
+LL |     foo::<{ BAR + 3 }>();
+   |           ^         ^
+
+error: expected one of `,` or `>`, found `-`
+  --> $DIR/const-expression-missing-braces.rs:20:15
+   |
+LL |     foo::<BAR - 3>();
+   |               ^ expected one of `,` or `>`
+   |
+help: to write a `const` expression, surround it with braces for it to be unambiguous
+   |
+LL |     foo::<{ BAR - 3 }>();
+   |           ^         ^
+
+error: expected one of `,` or `>`, found `-`
+  --> $DIR/const-expression-missing-braces.rs:24:15
+   |
+LL |     foo::<BAR - BAR>();
+   |               ^ expected one of `,` or `>`
+   |
+help: to write a `const` expression, surround it with braces for it to be unambiguous
+   |
+LL |     foo::<{ BAR - BAR }>();
+   |           ^           ^
 
 error: likely `const` expression parsed as trait bounds
   --> $DIR/const-expression-missing-braces.rs:13:11
    |
-LL |     foo::<bar + bar>();
+LL |     foo::<BAR + BAR>();
    |           ^^^^^^^^^ parsed as trait bounds but traits weren't found
    |
 help: if you meant to write a `const` expression, surround the expression with braces
    |
-LL |     foo::<{ bar + bar }>();
+LL |     foo::<{ BAR + BAR }>();
    |           ^           ^
 
-error: aborting due to 2 previous errors
+error: aborting due to 4 previous errors
 

src/test/ui/parser/removed-syntax-closure-lifetime.rs

@@ -1,2 +1,2 @@
 type closure = Box<lt/fn()>;
-//~^ ERROR expected one of `!`, `(`, `+`, `,`, `::`, `<`, or `>`, found `/`
+//~^ ERROR expected one of

src/test/ui/rfc-2497-if-let-chains/disallowed-positions.rs

@@ -232,7 +232,7 @@ fn inside_const_generic_arguments() {
     // In the cases above we have `ExprKind::Block` to help us out.
     // Below however, we would not have a block and so an implementation might go
     // from visiting expressions to types without banning `let` expressions down the tree.
-    // The parser admits non-IDENT expressions in const generic arguments and is caught by the 
+    // The parser admits non-IDENT expressions in const generic arguments and is caught by the
     // test below.
     if A::<
         true && let 1 = 1 //~ ERROR `let` expressions are not supported here