Rework generic args parsing

Author: estebank

src/librustc_resolve/late.rs

@@ -1494,14 +1494,15 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
         );
     }
 
-    fn smart_resolve_path_fragment(&mut self,
-                                   id: NodeId,
-                                   qself: Option<&QSelf>,
-                                   path: &[Segment],
-                                   span: Span,
-                                   source: PathSource<'_>,
-                                   crate_lint: CrateLint)
-                                   -> PartialRes {
+    fn smart_resolve_path_fragment(
+        &mut self,
+        id: NodeId,
+        qself: Option<&QSelf>,
+        path: &[Segment],
+        span: Span,
+        source: PathSource<'_>,
+        crate_lint: CrateLint,
+    ) -> PartialRes {
         let ns = source.namespace();
         let is_expected = &|res| source.is_expected(res);
 
@@ -1620,9 +1621,10 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
                 match self.resolve_qpath(id, qself, path, ns, span, crate_lint) {
                     // If defer_to_typeck, then resolution > no resolution,
                     // otherwise full resolution > partial resolution > no resolution.
-                    Some(partial_res) if partial_res.unresolved_segments() == 0 ||
-                                         defer_to_typeck =>
-                        return Some(partial_res),
+                    Some(partial_res)
+                    if partial_res.unresolved_segments() == 0 || defer_to_typeck => {
+                        return Some(partial_res)
+                    }
                     partial_res => if fin_res.is_none() { fin_res = partial_res },
                 }
             }

src/libsyntax/ast.rs

@@ -1253,32 +1253,6 @@ pub enum ExprKind {
     Err,
 }
 
-impl ExprKind {
-    /// Whether this expression can appear applied to a `const` parameter without being surrounded
-    /// by braces.
-    ///
-    /// Only used in error recovery.
-    crate fn is_valid_const_on_its_own(&self) -> bool {
-        fn is_const_lit(kind: &ExprKind) -> bool {
-            // These are the only literals that can be negated as a bare `const` argument.
-            match kind {
-                ExprKind::Lit(Lit { node: LitKind::Int(..), ..}) |
-                ExprKind::Lit(Lit { node: LitKind::Float(..), ..}) |
-                ExprKind::Lit(Lit { node: LitKind::FloatUnsuffixed(..), ..}) => true,
-                _ => false,
-            }
-        }
-
-        match self {
-            ExprKind::Lit(_) | // `foo::<42>()`
-            ExprKind::Err => true,
-            // `foo::<-42>()`
-            ExprKind::Unary(UnOp::Neg, expr) if is_const_lit(&expr.node) => true,
-            _ => false,
-        }
-    }
-}
-
 /// The explicit `Self` type in a "qualified path". The actual
 /// path, including the trait and the associated item, is stored
 /// separately. `position` represents the index of the associated

src/libsyntax/parse/parser.rs

@@ -40,7 +40,7 @@ bitflags::bitflags! {
     struct Restrictions: u8 {
         const STMT_EXPR           = 1 << 0;
         const NO_STRUCT_LITERAL   = 1 << 1;
-        const CONST_EXPR_RECOVERY = 1 << 2;
+        const CLOSING_ANGLE_BRACKET = 1 << 2;
     }
 }
 

src/libsyntax/parse/parser/expr.rs

@@ -186,9 +186,12 @@ impl<'a> Parser<'a> {
                 self.last_type_ascription = None;
                 return Ok(lhs);
             }
-            (true, Some(AssocOp::Greater)) if self.restrictions.contains(
-                Restrictions::CONST_EXPR_RECOVERY,
-            ) => { // Recovering likely const argument without braces.
+            (true, Some(AssocOp::Greater)) |
+            (true, Some(AssocOp::Less)) |
+            (true, Some(AssocOp::ShiftLeft)) |
+            (true, Some(AssocOp::ShiftRight))
+            if self.restrictions.contains(Restrictions::CLOSING_ANGLE_BRACKET) => {
+                // Bare `const` argument expression.
                 self.last_type_ascription = None;
                 return Ok(lhs);
             }
@@ -211,9 +214,13 @@ impl<'a> Parser<'a> {
         }
         self.expected_tokens.push(TokenType::Operator);
         while let Some(op) = AssocOp::from_token(&self.token) {
-            if let (AssocOp::Greater, true) = (&op, self.restrictions.contains(
-                Restrictions::CONST_EXPR_RECOVERY,
-            )) { // Recovering likely const argument without braces.
+            if let (true, AssocOp::Greater) |
+                (true, AssocOp::Less) |
+                (true, AssocOp::ShiftLeft) |
+                (true, AssocOp::ShiftRight) = (
+                self.restrictions.contains(Restrictions::CLOSING_ANGLE_BRACKET), &op
+            ) {
+                // Bare `const` argument expression fully parsed.
                 return Ok(lhs);
             }
 
@@ -352,12 +359,20 @@ impl<'a> Parser<'a> {
 
     /// Checks if this expression is a successfully parsed statement.
     fn expr_is_complete(&self, e: &Expr) -> bool {
-        (self.restrictions.contains(Restrictions::STMT_EXPR) &&
-            !classify::expr_requires_semi_to_be_stmt(e)) ||
-            (self.restrictions.contains(Restrictions::CONST_EXPR_RECOVERY) &&
-             // `<` is necessary here to avoid cases like `foo::< 1 < 3 >()` where we'll fallback
-             // to a regular parse error without recovery or suggestions.
-             [token::Lt, token::Gt, token::Comma].contains(&self.token.kind))
+        (
+            self.restrictions.contains(Restrictions::STMT_EXPR) &&
+            !classify::expr_requires_semi_to_be_stmt(e)
+        ) || (
+            self.restrictions.contains(Restrictions::CLOSING_ANGLE_BRACKET) &&
+            // Comparison and bitshift operators are not allowed in bare const expressions.
+            [
+                token::Lt,
+                token::Gt,
+                token::Comma,
+                token::BinOp(token::Shl),
+                token::BinOp(token::Shr),
+            ].contains(&self.token.kind)
+        )
     }
 
     fn is_at_start_of_range_notation_rhs(&self) -> bool {

src/libsyntax/parse/parser/path.rs

@@ -1,9 +1,9 @@
-use super::{P, Parser, PResult, Restrictions, TokenType};
+use super::{Parser, PResult, Restrictions, TokenType};
 
 use crate::{maybe_whole, ThinVec};
 use crate::ast::{
     self, AngleBracketedArgs, AnonConst, AssocTyConstraint, AssocTyConstraintKind, BlockCheckMode,
-    Expr, GenericArg, Ident, ParenthesizedArgs, Path, PathSegment, QSelf,
+    GenericArg, Ident, ParenthesizedArgs, Path, PathSegment, QSelf,
 };
 use crate::parse::token::{self, Token};
 use crate::source_map::{Span, BytePos};
@@ -323,7 +323,7 @@ impl<'a> Parser<'a> {
         };
 
         debug!("parse_generic_args_with_leading_angle_bracket_recovery: (snapshotting)");
-        match self.parse_generic_args() {
+        match self.parse_generic_args(style) {
             Ok(value) => Ok(value),
             Err(ref mut e) if is_first_invocation && self.unmatched_angle_bracket_count > 0 => {
                 // Cancel error from being unable to find `>`. We know the error
@@ -370,15 +370,18 @@ impl<'a> Parser<'a> {
                     .emit();
 
                 // Try again without unmatched angle bracket characters.
-                self.parse_generic_args()
+                self.parse_generic_args(style)
             },
             Err(e) => Err(e),
         }
     }
 
     /// Parses (possibly empty) list of lifetime and type arguments and associated type bindings,
     /// possibly including trailing comma.
-    fn parse_generic_args(&mut self) -> PResult<'a, (Vec<GenericArg>, Vec<AssocTyConstraint>)> {
+    fn parse_generic_args(
+        &mut self,
+        style: PathStyle,
+    ) -> PResult<'a, (Vec<GenericArg>, Vec<AssocTyConstraint>)> {
         let mut args = Vec::new();
         let mut constraints = Vec::new();
         let mut misplaced_assoc_ty_constraints: Vec<Span> = Vec::new();
@@ -415,59 +418,13 @@ impl<'a> Parser<'a> {
                     span,
                 });
                 assoc_ty_constraints.push(span);
-            } else if [
-                token::Not,
-                token::OpenDelim(token::Paren),
-            ].contains(&self.token.kind) && self.look_ahead(1, |t| t.is_lit() || t.is_bool_lit()) {
-                // Parse bad `const` argument. `!` is only allowed here to go through
-                // `recover_bare_const_expr` for better diagnostics when encountering
-                // `foo::<!false>()`. `(` is allowed for the case `foo::<(1, 2, 3)>()`.
-
-                // This can't possibly be a valid const arg, it is likely missing braces.
-                let value = AnonConst {
-                    id: ast::DUMMY_NODE_ID,
-                    value: self.recover_bare_const_expr()?,
-                };
-                args.push(GenericArg::Const(value));
+            } else if self.check_possible_const_needing_braces(style) {
+                args.push(self.parse_const_or_type_arg()?);
                 misplaced_assoc_ty_constraints.append(&mut assoc_ty_constraints);
             } else if self.check_const_arg() {
-                // Parse `const` argument.
-
-                // `const` arguments that don't require surrunding braces would have a length of
-                // one token, so anything that *isn't* surrounded by braces and is not
-                // immediately followed by `,` or `>` is not a valid `const` argument.
-                let invalid = self.look_ahead(1, |t| t != &token::Lt && t != &token::Comma);
-
-                let expr = if let token::OpenDelim(token::Brace) = self.token.kind {
-                    // Parse `const` argument surrounded by braces.
-                    self.parse_block_expr(
-                        None, self.token.span, BlockCheckMode::Default, ThinVec::new()
-                    )?
-                } else if invalid {
-                    // This can't possibly be a valid const arg, it is likely missing braces.
-                    self.recover_bare_const_expr()?
-                } else if self.token.is_ident() {
-                    // FIXME(const_generics): to distinguish between idents for types and consts,
-                    // we should introduce a GenericArg::Ident in the AST and distinguish when
-                    // lowering to the HIR. For now, idents for const args are not permitted.
-                    if self.token.is_bool_lit() {
-                        self.parse_literal_maybe_minus()?
-                    } else {
-                        return Err(
-                            self.fatal("identifiers may currently not be used for const generics")
-                        );
-                    }
-                } else {
-                    self.parse_literal_maybe_minus()?
-                };
-                let value = AnonConst {
-                    id: ast::DUMMY_NODE_ID,
-                    value: expr,
-                };
-                args.push(GenericArg::Const(value));
+                args.push(self.parse_const_arg()?);
                 misplaced_assoc_ty_constraints.append(&mut assoc_ty_constraints);
             } else if self.check_type() {
-                // Parse type argument.
                 args.push(GenericArg::Type(self.parse_ty()?));
                 misplaced_assoc_ty_constraints.append(&mut assoc_ty_constraints);
             } else {
@@ -499,49 +456,86 @@ impl<'a> Parser<'a> {
         Ok((args, constraints))
     }
 
-    fn recover_bare_const_expr(&mut self) -> PResult<'a, P<Expr>> {
-        let snapshot = self.clone();
-        debug!("recover_bare_const_expr {:?}", self.token);
-        match self.parse_expr_res(Restrictions::CONST_EXPR_RECOVERY, None) {
-            Ok(expr) => {
-                debug!("recover_bare_const_expr expr {:?} {:?}", expr, expr.node);
-                if let token::Comma | token::Gt = self.token.kind {
-                    // We parsed the whole const argument successfully without braces.
-                    debug!("recover_bare_const_expr ok");
-                    if !expr.node.is_valid_const_on_its_own() {
-                        // But it wasn't a literal, so we emit a custom error and
-                        // suggest the appropriate code. `foo::<-1>()` is valid but gets parsed
-                        // here, so we need to gate the error only for invalid cases.
-                        self.span_fatal(
-                            expr.span,
-                            "complex const arguments must be surrounded by braces",
-                        ).multipart_suggestion(
-                            "surround this const argument in braces",
-                            vec![
-                                (expr.span.shrink_to_lo(), "{ ".to_string()),
-                                (expr.span.shrink_to_hi(), " }".to_string()),
-                            ],
-                            Applicability::MachineApplicable,
-                        ).emit();
+    fn parse_const_arg(&mut self) -> PResult<'a, GenericArg> {
+        let value = if let token::OpenDelim(token::Brace) = self.token.kind {
+            // Parse `const` argument surrounded by braces.
+            self.parse_block_expr(None, self.token.span, BlockCheckMode::Default, ThinVec::new())?
+        } else {
+            self.parse_expr_res(Restrictions::CLOSING_ANGLE_BRACKET, None)?
+        };
+        let value = AnonConst { id: ast::DUMMY_NODE_ID, value };
+        Ok(GenericArg::Const(value))
+    }
+
+    /// Check some ambiguous cases between type and non-block const arguments.
+    fn check_possible_const_needing_braces(&mut self, style: PathStyle) -> bool {
+        style == PathStyle::Expr && (
+            self.token.kind == token::Not || // `foo::<!const_bool_fn()>()`
+            self.token.kind == token::OpenDelim(token::Paren) // `foo::<(1, 2, 3)>()`
+        )
+    }
+
+    /// There's intent ambiguity for this argument, it could be a const expression missing braces,
+    /// or it could be a type argument. We try the later as the grammar expects, and if it fails we
+    /// attempt the former and emit a targetted suggestion if valid.
+    fn parse_const_or_type_arg(&mut self) -> PResult<'a, GenericArg> {
+        let mut snapshot = self.clone();
+        match self.parse_ty() {
+            // Nothing to do, this was indeed a type argument.
+            Ok(ty) if [
+                token::Comma,
+                token::Gt,
+                token::BinOp(token::Shr),
+            ].contains(&self.token.kind) => Ok(GenericArg::Type(ty)),
+            Ok(ty) => { // Could have found `foo::<!false>()`, needs `foo::<{ !false }>()`.
+                mem::swap(self, &mut snapshot);
+                match self.parse_bad_const_arg() {
+                    Ok(arg) => Ok(arg),
+                    Err(mut err) => {
+                        mem::swap(self, &mut snapshot);
+                        err.cancel();
+                        Ok(GenericArg::Type(ty))
+                    }
+                }
+            }
+            Err(mut ty_err) => {
+                mem::swap(self, &mut snapshot);
+                match self.parse_bad_const_arg() {
+                    Ok(arg) => {
+                        ty_err.cancel();
+                        Ok(arg)
+                    }
+                    Err(mut err) => {
+                        mem::swap(self, &mut snapshot);
+                        err.cancel();
+                        Err(ty_err)
                     }
-                    Ok(expr)
-                } else {
-                    debug!("recover_bare_const_expr not");
-                    // We parsed *some* expression, but it isn't the whole argument
-                    // so we can't ensure it was a const argument with missing braces.
-                    // Roll-back and emit a regular parser error.
-                    mem::replace(self, snapshot);
-                    self.parse_literal_maybe_minus()
                 }
             }
-            Err(mut err) => {
-                debug!("recover_bare_const_expr err");
-                // We couldn't parse an expression successfully.
-                // Roll-back, hide the error and emit a regular parser error.
-                err.cancel();
-                mem::replace(self, snapshot);
-                self.parse_literal_maybe_minus()
+        }
+    }
+
+    fn parse_bad_const_arg(&mut self) -> PResult<'a, GenericArg> {
+        let msg = if self.token.kind == token::OpenDelim(token::Paren) {
+            // `foo::<(1, 2, 3)>()`
+            "tuples in const arguments must be surrounded by braces"
+        } else {
+            "complex const arguments must be surrounded by braces"
+        };
+        match self.parse_const_arg() {
+            Ok(arg) => {
+                self.span_fatal(arg.span(), msg)
+                    .multipart_suggestion(
+                        "surround this const argument in braces",
+                        vec![
+                            (arg.span().shrink_to_lo(), "{ ".to_string()),
+                            (arg.span().shrink_to_hi(), " }".to_string()),
+                        ],
+                        Applicability::MachineApplicable,
+                    ).emit();
+                Ok(arg)
             }
+            Err(err) => Err(err),
         }
     }
 }

src/libsyntax/parse/parser/ty.rs

@@ -48,8 +48,12 @@ impl<'a> Parser<'a> {
         }
     }
 
-    pub(super) fn parse_ty_common(&mut self, allow_plus: bool, allow_qpath_recovery: bool,
-                       allow_c_variadic: bool) -> PResult<'a, P<Ty>> {
+    pub(super) fn parse_ty_common(
+        &mut self,
+        allow_plus: bool,
+        allow_qpath_recovery: bool,
+        allow_c_variadic: bool,
+    ) -> PResult<'a, P<Ty>> {
         maybe_recover_from_interpolated_ty_qpath!(self, allow_qpath_recovery);
         maybe_whole!(self, NtTy, |x| x);