Review comments

- generate error instead of warning - remove `RewindPoint` and just keep a copy of `Parser` to rewind state. - `dont_parse_generics: bool` -> `parse_generics: bool` - remove `eat_lt` - move error handling code to separate method
rust-lang · Jun 15, 2017 · ad260ff · ad260ff
1 parent 46a6af1
commit ad260ff
Show file tree

Hide file tree

Showing 10 changed files with 83 additions and 109 deletions.
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
@@ -150,7 +150,7 @@ fn maybe_append(mut lhs: Vec<Attribute>, rhs: Option<Vec<Attribute>>)
     lhs
 }
 
-#[derive(PartialEq)]
+#[derive(Clone, PartialEq)]
 enum PrevTokenKind {
     DocComment,
     Comma,
@@ -162,6 +162,7 @@ enum PrevTokenKind {
 
 /* ident is handled by common.rs */
 
+#[derive(Clone)]
 pub struct Parser<'a> {
     pub sess: &'a ParseSess,
     /// the current token:
@@ -441,15 +442,6 @@ fn dummy_arg(span: Span) -> Arg {
     Arg { ty: P(ty), pat: pat, id: ast::DUMMY_NODE_ID }
 }
 
-struct RewindPoint {
-    token: token::Token,
-    span: Span,
-    meta_var_span: Option<Span>,
-    prev_span: Span,
-    token_cursor: TokenCursor,
-    expected_tokens: Vec<TokenType>,
-}
-
 impl<'a> Parser<'a> {
     pub fn new(sess: &'a ParseSess,
                tokens: TokenStream,
@@ -798,13 +790,6 @@ impl<'a> Parser<'a> {
         }
     }
 
-    fn is_lt(&mut self) -> bool {
-        match self.token {
-            token::Lt | token::BinOp(token::Shl) => true,
-            _ => false,
-        }
-    }
-
     /// Attempt to consume a `<`. If `<<` is seen, replace it with a single
     /// `<` and continue. If a `<` is not seen, return false.
     ///
@@ -1743,7 +1728,7 @@ impl<'a> Parser<'a> {
 
         let segments = match mode {
             PathStyle::Type => {
-                self.parse_path_segments_without_colons(false)?
+                self.parse_path_segments_without_colons(true)?
             }
             PathStyle::Expr => {
                 self.parse_path_segments_with_colons()?
@@ -1764,14 +1749,14 @@ impl<'a> Parser<'a> {
     /// bounds are permitted and whether `::` must precede type parameter
     /// groups.
     pub fn parse_path(&mut self, mode: PathStyle) -> PResult<'a, ast::Path> {
-        self.parse_path_common(mode, false)
+        self.parse_path_common(mode, true)
     }
 
     pub fn parse_path_without_generics(&mut self, mode: PathStyle) -> PResult<'a, ast::Path> {
-        self.parse_path_common(mode, true)
+        self.parse_path_common(mode, false)
     }
 
-    fn parse_path_common(&mut self, mode: PathStyle, dont_parse_generics: bool)
+    fn parse_path_common(&mut self, mode: PathStyle, parse_generics: bool)
         -> PResult<'a, ast::Path>
     {
         maybe_whole!(self, NtPath, |x| x);
@@ -1784,7 +1769,7 @@ impl<'a> Parser<'a> {
         // A bound set is a set of type parameter bounds.
         let mut segments = match mode {
             PathStyle::Type => {
-                self.parse_path_segments_without_colons(dont_parse_generics)?
+                self.parse_path_segments_without_colons(parse_generics)?
             }
             PathStyle::Expr => {
                 self.parse_path_segments_with_colons()?
@@ -1829,7 +1814,7 @@ impl<'a> Parser<'a> {
     /// - `a::b<T,U>::c<V,W>`
     /// - `a::b<T,U>::c(V) -> W`
     /// - `a::b<T,U>::c(V)`
-    pub fn parse_path_segments_without_colons(&mut self, dont_parse_generics: bool)
+    pub fn parse_path_segments_without_colons(&mut self, parse_generics: bool)
         -> PResult<'a, Vec<PathSegment>>
     {
         let mut segments = Vec::new();
@@ -1850,8 +1835,7 @@ impl<'a> Parser<'a> {
             }
 
             // Parse types, optionally.
-            let parameters = if self.is_lt() && !dont_parse_generics {
-                let _ = self.eat_lt();
+            let parameters = if parse_generics && self.eat_lt() {
                 let (lifetimes, types, bindings) = self.parse_generic_args()?;
                 self.expect_gt()?;
                 ast::AngleBracketedParameterData {
@@ -2832,60 +2816,7 @@ impl<'a> Parser<'a> {
             if op == AssocOp::As {
                 // Save the state of the parser before parsing type normally, in case there is a
                 // LessThan comparison after this cast.
-                let rp = self.get_rewind_point();
-                match self.parse_ty_no_plus() {
-                    Ok(rhs) => {
-                        lhs = self.mk_expr(lhs_span.to(rhs.span),
-                                           ExprKind::Cast(lhs, rhs), ThinVec::new());
-                    }
-                    Err(mut err) => {
-                        // Rewind to before attempting to parse the type with generics, to get
-                        // arround #22644.
-                        let rp_err = self.get_rewind_point();
-                        let sp = rp_err.span.clone();
-                        self.rewind(rp);
-                        let lo = self.span;
-                        let path = match self.parse_path_without_generics(PathStyle::Type) {
-                            Ok(path) => {
-                                // Successfully parsed the type leaving a `<` yet to parse
-                                err.cancel();
-                                let codemap = self.sess.codemap();
-                                let suggestion_span = lhs_span.to(self.prev_span);
-                                let suggestion = match codemap.span_to_snippet(suggestion_span) {
-                                    Ok(lstring) => format!("({})", lstring),
-                                    _ => format!("(<expression>)")
-                                };
-                                let warn_message = match codemap.span_to_snippet(self.prev_span) {
-                                    Ok(lstring) => format!("`{}`", lstring),
-                                    _ => "a type".to_string(),
-                                };
-                                let msg = format!("`<` is interpreted as a start of generic \
-                                                   arguments for {}, not a comparison",
-                                                  warn_message);
-                                let mut warn = self.sess.span_diagnostic.struct_span_warn(sp, &msg);
-                                warn.span_label(sp, "interpreted as generic argument");
-                                warn.span_label(self.span, "not interpreted as comparison");
-                                warn.span_suggestion(suggestion_span,
-                                                    "if you want to compare the casted value \
-                                                     then write:",
-                                                    suggestion);
-                                warn.emit();
-                                path
-                            }
-                            Err(mut path_err) => {
-                                // Still couldn't parse, return original error and parser state
-                                path_err.cancel();
-                                self.rewind(rp_err);
-                                return Err(err);
-                            }
-                        };
-                        let path = TyKind::Path(None, path);
-                        let span = lo.to(self.prev_span);
-                        let rhs = P(Ty { node: path, span: span, id: ast::DUMMY_NODE_ID });
-                        lhs = self.mk_expr(lhs_span.to(rhs.span),
-                                           ExprKind::Cast(lhs, rhs), ThinVec::new());
-                    }
-                };
+                lhs = self.parse_assoc_op_as(lhs, lhs_span)?;
                 continue
             } else if op == AssocOp::Colon {
                 let rhs = self.parse_ty_no_plus()?;
@@ -2983,6 +2914,67 @@ impl<'a> Parser<'a> {
         Ok(lhs)
     }
 
+    fn parse_assoc_op_as(&mut self, lhs: P<Expr>, lhs_span: Span) -> PResult<'a, P<Expr>> {
+        let rp = self.clone();
+        match self.parse_ty_no_plus() {
+            Ok(rhs) => {
+                Ok(self.mk_expr(lhs_span.to(rhs.span),
+                                ExprKind::Cast(lhs, rhs),
+                                ThinVec::new()))
+            }
+            Err(mut err) => {
+                let rp_err = self.clone();
+                let sp = rp_err.span.clone();
+
+                // Rewind to before attempting to parse the type with generics, to get
+                // arround #22644.
+                mem::replace(self, rp);
+                let lo = self.span;
+                match self.parse_path_without_generics(PathStyle::Type) {
+                    Ok(path) => {
+                        // Successfully parsed the type leaving a `<` yet to parse
+                        err.cancel();
+                        let codemap = self.sess.codemap();
+                        let suggestion_span = lhs_span.to(self.prev_span);
+                        let suggestion = match codemap.span_to_snippet(suggestion_span) {
+                            Ok(lstring) => format!("({})", lstring),
+                            _ => format!("(<expression> as <type>)")
+                        };
+                        let warn_message = match codemap.span_to_snippet(self.prev_span) {
+                            Ok(lstring) => format!("`{}`", lstring),
+                            _ => "a type".to_string(),
+                        };
+                        let msg = format!("`<` is interpreted as a start of generic \
+                                           arguments for {}, not a comparison",
+                                          warn_message);
+                        let mut err = self.sess.span_diagnostic.struct_span_err(sp, &msg);
+                        err.span_label(sp, "interpreted as generic argument");
+                        err.span_label(self.span, "not interpreted as comparison");
+                        err.span_suggestion(suggestion_span,
+                                            "if you want to compare the casted value then write:",
+                                            suggestion);
+                        err.emit();
+
+                        let path = TyKind::Path(None, path);
+                        let span = lo.to(self.prev_span);
+                        let rhs = P(Ty { node: path, span: span, id: ast::DUMMY_NODE_ID });
+                        // Letting the parser accept the recovered type to avoid further errors,
+                        // but the code will still not compile due to the error emitted above.
+                        Ok(self.mk_expr(lhs_span.to(rhs.span),
+                                        ExprKind::Cast(lhs, rhs),
+                                        ThinVec::new()))
+                    }
+                    Err(mut path_err) => {
+                        // Still couldn't parse, return original error and parser state
+                        path_err.cancel();
+                        mem::replace(self, rp_err);
+                        Err(err)
+                    }
+                }
+            }
+        }
+    }
+
     /// Produce an error if comparison operators are chained (RFC #558).
     /// We only need to check lhs, not rhs, because all comparison ops
     /// have same precedence and are left-associative
@@ -6264,24 +6256,4 @@ impl<'a> Parser<'a> {
             _ =>  Err(self.fatal("expected string literal"))
         }
     }
-
-    fn get_rewind_point(&mut self) -> RewindPoint {
-        RewindPoint {
-            token: self.token.clone(),
-            span: self.span,
-            meta_var_span: self.meta_var_span,
-            prev_span: self.prev_span,
-            token_cursor: self.token_cursor.clone(),
-            expected_tokens: self.expected_tokens.clone(),
-        }
-    }
-
-    fn rewind(&mut self, rp: RewindPoint) {
-        self.token = rp.token;
-        self.span = rp.span;
-        self.meta_var_span = rp.meta_var_span;
-        self.prev_span = rp.prev_span;
-        self.token_cursor = rp.token_cursor;
-        self.expected_tokens = rp.expected_tokens;
-    }
 }
diff --git a/src/test/parse-fail/better-expected.rs b/src/test/parse-fail/better-expected.rs
@@ -11,5 +11,5 @@
 // compile-flags: -Z parse-only
 
 fn main() {
-    let x: [isize 3]; //~ ERROR expected one of `!`, `(`, `+`, `::`, `;`, or `]`, found `3`
+    let x: [isize 3]; //~ ERROR expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `3`
 }
diff --git a/src/test/parse-fail/bounds-type-where.rs b/src/test/parse-fail/bounds-type-where.rs
@@ -18,6 +18,6 @@ type A where T: Trait + Trait = u8; // OK
 type A where = u8; // OK
 type A where T: Trait + = u8; // OK
 type A where T, = u8;
-//~^ ERROR expected one of `!`, `(`, `+`, `::`, `:`, `==`, or `=`, found `,`
+//~^ ERROR expected one of `!`, `(`, `+`, `::`, `:`, `<`, `==`, or `=`, found `,`
 
 fn main() {}
diff --git a/src/test/parse-fail/closure-return-syntax.rs b/src/test/parse-fail/closure-return-syntax.rs
@@ -13,5 +13,5 @@
 
 fn main() {
     let x = || -> i32 22;
-    //~^ ERROR expected one of `!`, `(`, `::`, or `{`, found `22`
+    //~^ ERROR expected one of `!`, `(`, `::`, `<`, or `{`, found `22`
 }
diff --git a/src/test/parse-fail/empty-impl-semicolon.rs b/src/test/parse-fail/empty-impl-semicolon.rs
@@ -10,4 +10,4 @@
 
 // compile-flags: -Z parse-only
 
-impl Foo; //~ ERROR expected one of `!`, `(`, `+`, `::`, `for`, `where`, or `{`, found `;`
+impl Foo; //~ ERROR expected one of `!`, `(`, `+`, `::`, `<`, `for`, `where`, or `{`, found `;`
diff --git a/src/test/parse-fail/multitrait.rs b/src/test/parse-fail/multitrait.rs
@@ -15,7 +15,7 @@ struct S {
 }
 
 impl Cmp, ToString for S {
-//~^ ERROR: expected one of `!`, `(`, `+`, `::`, `for`, `where`, or `{`, found `,`
+//~^ ERROR: expected one of `!`, `(`, `+`, `::`, `<`, `for`, `where`, or `{`, found `,`
   fn eq(&&other: S) { false }
   fn to_string(&self) -> String { "hi".to_string() }
 }
diff --git a/src/test/parse-fail/removed-syntax-closure-lifetime.rs b/src/test/parse-fail/removed-syntax-closure-lifetime.rs
@@ -11,4 +11,4 @@
 // compile-flags: -Z parse-only
 
 type closure = Box<lt/fn()>;
-//~^ ERROR expected one of `!`, `(`, `+`, `,`, `::`, or `>`, found `/`
+//~^ ERROR expected one of `!`, `(`, `+`, `,`, `::`, `<`, or `>`, found `/`
diff --git a/src/test/parse-fail/removed-syntax-fixed-vec.rs b/src/test/parse-fail/removed-syntax-fixed-vec.rs
@@ -10,4 +10,4 @@
 
 // compile-flags: -Z parse-only
 
-type v = [isize * 3]; //~ ERROR expected one of `!`, `(`, `+`, `::`, `;`, or `]`, found `*`
+type v = [isize * 3]; //~ ERROR expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `*`
diff --git a/src/test/parse-fail/removed-syntax-ptr-lifetime.rs b/src/test/parse-fail/removed-syntax-ptr-lifetime.rs
@@ -10,4 +10,4 @@
 
 // compile-flags: -Z parse-only
 
-type bptr = &lifetime/isize; //~ ERROR expected one of `!`, `(`, `::`, or `;`, found `/`
+type bptr = &lifetime/isize; //~ ERROR expected one of `!`, `(`, `::`, `;`, or `<`, found `/`
diff --git a/src/test/ui/issue-22644.stderr b/src/test/ui/issue-22644.stderr
@@ -1,4 +1,4 @@
-warning: `<` is interpreted as a start of generic arguments for `usize`, not a comparison
+error: `<` is interpreted as a start of generic arguments for `usize`, not a comparison
   --> $DIR/issue-22644.rs:16:33
    |
 16 |     println!("{}", a as usize < b);
@@ -9,7 +9,7 @@ warning: `<` is interpreted as a start of generic arguments for `usize`, not a c
 help: if you want to compare the casted value then write:
    |     println!("{}", (a as usize) < b);
 
-warning: `<` is interpreted as a start of generic arguments for `usize`, not a comparison
+error: `<` is interpreted as a start of generic arguments for `usize`, not a comparison
   --> $DIR/issue-22644.rs:17:33
    |
 17 |     println!("{}", a as usize < 4);
@@ -20,3 +20,5 @@ warning: `<` is interpreted as a start of generic arguments for `usize`, not a c
 help: if you want to compare the casted value then write:
    |     println!("{}", (a as usize) < 4);
 
+error: aborting due to previous error(s)
+