From a6d32153a6a70390e1c0c24907179d1e2a1f1ef9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 9 Jun 2017 20:30:33 -0700 Subject: [PATCH 1/5] Learn to parse `a as usize < b` Parsing `a as usize > b` always works, but `a as usize < b` was a parsing error because the parser would think the `<` started a generic type argument for `usize`. The parser now attempts to parse as before, and if a DiagnosticError is returned, try to parse again as a type with no generic arguments. If this fails, return the original `DiagnosticError`. --- src/libsyntax/parse/parser.rs | 98 +++++++++++++++++++++++++++++--- src/libsyntax/tokenstream.rs | 3 + src/test/run-pass/issue-22644.rs | 18 ++++++ 3 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 src/test/run-pass/issue-22644.rs diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index d9cb2b4ab7db0..a6ecd304dbd16 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -193,11 +193,13 @@ pub struct Parser<'a> { } +#[derive(Clone)] struct TokenCursor { frame: TokenCursorFrame, stack: Vec, } +#[derive(Clone)] struct TokenCursorFrame { delim: token::DelimToken, span: Span, @@ -397,6 +399,7 @@ impl Error { } } +#[derive(Debug)] pub enum LhsExpr { NotYetParsed, AttributesParsed(ThinVec), @@ -438,6 +441,8 @@ fn dummy_arg(span: Span) -> Arg { Arg { ty: P(ty), pat: pat, id: ast::DUMMY_NODE_ID } } +type RewindPoint = (token::Token, Span, Option, Span, TokenCursor, Vec); + impl<'a> Parser<'a> { pub fn new(sess: &'a ParseSess, tokens: TokenStream, @@ -786,6 +791,13 @@ 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. /// @@ -1724,7 +1736,7 @@ impl<'a> Parser<'a> { let segments = match mode { PathStyle::Type => { - self.parse_path_segments_without_colons()? + self.parse_path_segments_without_colons(false)? } PathStyle::Expr => { self.parse_path_segments_with_colons()? @@ -1745,6 +1757,16 @@ 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) + } + + pub fn parse_path_without_generics(&mut self, mode: PathStyle) -> PResult<'a, ast::Path> { + self.parse_path_common(mode, true) + } + + fn parse_path_common(&mut self, mode: PathStyle, dont_parse_generics: bool) + -> PResult<'a, ast::Path> + { maybe_whole!(self, NtPath, |x| x); let lo = self.meta_var_span.unwrap_or(self.span); @@ -1755,7 +1777,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()? + self.parse_path_segments_without_colons(dont_parse_generics)? } PathStyle::Expr => { self.parse_path_segments_with_colons()? @@ -1800,7 +1822,9 @@ impl<'a> Parser<'a> { /// - `a::b::c` /// - `a::b::c(V) -> W` /// - `a::b::c(V)` - pub fn parse_path_segments_without_colons(&mut self) -> PResult<'a, Vec> { + pub fn parse_path_segments_without_colons(&mut self, dont_parse_generics: bool) + -> PResult<'a, Vec> + { let mut segments = Vec::new(); loop { // First, parse an identifier. @@ -1819,7 +1843,8 @@ impl<'a> Parser<'a> { } // Parse types, optionally. - let parameters = if self.eat_lt() { + let parameters = if self.is_lt() && !dont_parse_generics { + let _ = self.eat_lt(); let (lifetimes, types, bindings) = self.parse_generic_args()?; self.expect_gt()?; ast::AngleBracketedParameterData { @@ -2798,8 +2823,40 @@ impl<'a> Parser<'a> { } // Special cases: if op == AssocOp::As { - let rhs = self.parse_ty_no_plus()?; - lhs = self.mk_expr(lhs_span.to(rhs.span), ExprKind::Cast(lhs, rhs), ThinVec::new()); + // 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(); + 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(); + 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()); + } + }; continue } else if op == AssocOp::Colon { let rhs = self.parse_ty_no_plus()?; @@ -2901,7 +2958,9 @@ impl<'a> Parser<'a> { /// We only need to check lhs, not rhs, because all comparison ops /// have same precedence and are left-associative fn check_no_chained_comparison(&mut self, lhs: &Expr, outer_op: &AssocOp) { - debug_assert!(outer_op.is_comparison()); + debug_assert!(outer_op.is_comparison(), + "check_no_chained_comparison: {:?} is not comparison", + outer_op); match lhs.node { ExprKind::Binary(op, _, _) if op.node.is_comparison() => { // respan to include both operators @@ -2925,7 +2984,9 @@ impl<'a> Parser<'a> { fn parse_prefix_range_expr(&mut self, already_parsed_attrs: Option>) -> PResult<'a, P> { - debug_assert!(self.token == token::DotDot || self.token == token::DotDotDot); + debug_assert!(self.token == token::DotDot || self.token == token::DotDotDot, + "parse_prefix_range_expr: token {:?} is not DotDot or DotDotDot", + self.token); let tok = self.token.clone(); let attrs = self.parse_or_use_outer_attributes(already_parsed_attrs)?; let lo = self.span; @@ -6174,4 +6235,25 @@ impl<'a> Parser<'a> { _ => Err(self.fatal("expected string literal")) } } + + fn get_rewind_point(&mut self) -> RewindPoint { + ( + self.token.clone(), + self.span, + self.meta_var_span, + self.prev_span, + self.token_cursor.clone(), + self.expected_tokens.clone(), + ) + } + + fn rewind(&mut self, rp: RewindPoint) { + let (token, span, meta_var_span, prev_span, token_cursor, expected_tokens,) = rp; + self.token = token; + self.span = span; + self.meta_var_span = meta_var_span; + self.prev_span = prev_span; + self.token_cursor = token_cursor; + self.expected_tokens = expected_tokens; + } } diff --git a/src/libsyntax/tokenstream.rs b/src/libsyntax/tokenstream.rs index 339e7c0b628ad..963482fc223f1 100644 --- a/src/libsyntax/tokenstream.rs +++ b/src/libsyntax/tokenstream.rs @@ -227,14 +227,17 @@ impl TokenStream { } } +#[derive(Clone)] pub struct Cursor(CursorKind); +#[derive(Clone)] enum CursorKind { Empty, Tree(TokenTree, bool /* consumed? */), Stream(StreamCursor), } +#[derive(Clone)] struct StreamCursor { stream: RcSlice, index: usize, diff --git a/src/test/run-pass/issue-22644.rs b/src/test/run-pass/issue-22644.rs new file mode 100644 index 0000000000000..9269180396c54 --- /dev/null +++ b/src/test/run-pass/issue-22644.rs @@ -0,0 +1,18 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + let a : u32 = 0; + let b : usize = 0; + + println!("{}", a as usize > b); + println!("{}", a as usize < b); + println!("{}", a as usize < 4); +} From 3a7dbf48feb325bbe8517bc0fd7546e80931c8ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 11 Jun 2017 23:47:26 -0700 Subject: [PATCH 2/5] Suggest non-ambiguous comparison after cast ``` warning: `<` is interpreted as a start of generic arguments for `usize`, not comparison --> $DIR/issue-22644.rs:16:33 | 16 | println!("{}", a as usize < b); | ^ expected one of `!`, `(`, `+`, `,`, `::`, or `>` here | help: if you want to compare the casted value then write | println!("{}", (a as usize) < b); ``` --- src/librustc_errors/diagnostic.rs | 4 ++++ src/libsyntax/parse/parser.rs | 21 +++++++++++++++++++-- src/test/{run-pass => ui}/issue-22644.rs | 0 src/test/ui/issue-22644.stderr | 20 ++++++++++++++++++++ 4 files changed, 43 insertions(+), 2 deletions(-) rename src/test/{run-pass => ui}/issue-22644.rs (100%) create mode 100644 src/test/ui/issue-22644.stderr diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index 7a64cdeee65c7..d7c21127474a4 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -248,6 +248,10 @@ impl Diagnostic { self.message.iter().map(|i| i.0.to_owned()).collect::() } + pub fn set_message(&mut self, message: &str) { + self.message = vec![(message.to_owned(), Style::NoStyle)]; + } + pub fn styled_message(&self) -> &Vec<(String, Style)> { &self.message } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index a6ecd304dbd16..76b14071ec321 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -42,7 +42,7 @@ use ast::RangeEnd; use {ast, attr}; use codemap::{self, CodeMap, Spanned, respan}; use syntax_pos::{self, Span, BytePos}; -use errors::{self, DiagnosticBuilder}; +use errors::{self, DiagnosticBuilder, Level}; use parse::{self, classify, token}; use parse::common::SeqSep; use parse::lexer::TokenAndSpan; @@ -2840,7 +2840,24 @@ impl<'a> Parser<'a> { 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!("()") + }; + let warn_message = match codemap.span_to_snippet(self.prev_span) { + Ok(lstring) => format!("`{}`", lstring), + _ => "a type".to_string(), + }; + err.span_suggestion(suggestion_span, + "if you want to compare the casted value then write", + suggestion); + err.level = Level::Warning; + err.set_message(&format!("`<` is interpreted as a start of generic \ + arguments for {}, not a comparison", + warn_message)); + err.emit(); path } Err(mut path_err) => { diff --git a/src/test/run-pass/issue-22644.rs b/src/test/ui/issue-22644.rs similarity index 100% rename from src/test/run-pass/issue-22644.rs rename to src/test/ui/issue-22644.rs diff --git a/src/test/ui/issue-22644.stderr b/src/test/ui/issue-22644.stderr new file mode 100644 index 0000000000000..1e85daa577800 --- /dev/null +++ b/src/test/ui/issue-22644.stderr @@ -0,0 +1,20 @@ +warning: `<` is interpreted as a start of generic arguments for `usize`, not comparison + --> $DIR/issue-22644.rs:16:33 + | +16 | println!("{}", a as usize < b); + | ^ expected one of `!`, `(`, `+`, `,`, `::`, or `>` here + | +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 comparison + --> $DIR/issue-22644.rs:17:33 + | +17 | println!("{}", a as usize < 4); + | -^ unexpected token + | | + | expected one of `>`, identifier, lifetime, or type here + | +help: if you want to compare the casted value then write + | println!("{}", (a as usize) < 4); + From 5fcfa08e9899044e5cad2ac238206d83568a863f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 11 Jun 2017 23:49:28 -0700 Subject: [PATCH 3/5] Fix affected tests --- src/libsyntax/parse/parser.rs | 3 ++- src/test/parse-fail/better-expected.rs | 2 +- src/test/parse-fail/bounds-type-where.rs | 2 +- src/test/parse-fail/closure-return-syntax.rs | 2 +- src/test/parse-fail/empty-impl-semicolon.rs | 2 +- src/test/parse-fail/multitrait.rs | 2 +- src/test/parse-fail/removed-syntax-closure-lifetime.rs | 2 +- src/test/parse-fail/removed-syntax-fixed-vec.rs | 2 +- src/test/parse-fail/removed-syntax-ptr-lifetime.rs | 2 +- src/test/ui/issue-22644.stderr | 8 ++++---- 10 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 76b14071ec321..ee74d9ec2e656 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2851,7 +2851,8 @@ impl<'a> Parser<'a> { _ => "a type".to_string(), }; err.span_suggestion(suggestion_span, - "if you want to compare the casted value then write", + "if you want to compare the casted value \ + then write:", suggestion); err.level = Level::Warning; err.set_message(&format!("`<` is interpreted as a start of generic \ diff --git a/src/test/parse-fail/better-expected.rs b/src/test/parse-fail/better-expected.rs index b60201c251dcc..d78ac3cb5ce90 100644 --- 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 index 9dc5d8277446d..1037854959331 100644 --- 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 index 1da6735918012..f3be750e25462 100644 --- 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 index 9939f1e36ea9d..ef25e5d0fc2e4 100644 --- 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 index b7c9b16588466..d2be89d19ecee 100644 --- 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 index b305b1894a810..2dd2bd22d1a3d 100644 --- 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; -//~^ 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 index 0f34db0885202..f943d5d50cac4 100644 --- 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 index b91ab8730b3dc..40f368e710543 100644 --- 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 index 1e85daa577800..2cbb42653565d 100644 --- a/src/test/ui/issue-22644.stderr +++ b/src/test/ui/issue-22644.stderr @@ -1,13 +1,13 @@ -warning: `<` is interpreted as a start of generic arguments for `usize`, not comparison +warning: `<` 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); | ^ expected one of `!`, `(`, `+`, `,`, `::`, or `>` here | -help: if you want to compare the casted value then write +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 comparison +warning: `<` 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); @@ -15,6 +15,6 @@ warning: `<` is interpreted as a start of generic arguments for `usize`, not com | | | expected one of `>`, identifier, lifetime, or type here | -help: if you want to compare the casted value then write +help: if you want to compare the casted value then write: | println!("{}", (a as usize) < 4); From 46a6af12aa8d688aa52279647b01be6035adf22c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 12 Jun 2017 10:22:08 -0700 Subject: [PATCH 4/5] Change `<` interpreted as generic arg start warning ``` warning: `<` 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); | - ^ interpreted as generic argument | | | not interpreted as comparison | help: if you want to compare the casted value then write: | println!("{}", (a as usize) < b); ``` --- src/libsyntax/parse/parser.rs | 56 ++++++++++++++++++++-------------- src/test/ui/issue-22644.stderr | 10 +++--- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ee74d9ec2e656..b3623fbe3ac1a 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -42,7 +42,7 @@ use ast::RangeEnd; use {ast, attr}; use codemap::{self, CodeMap, Spanned, respan}; use syntax_pos::{self, Span, BytePos}; -use errors::{self, DiagnosticBuilder, Level}; +use errors::{self, DiagnosticBuilder}; use parse::{self, classify, token}; use parse::common::SeqSep; use parse::lexer::TokenAndSpan; @@ -441,7 +441,14 @@ fn dummy_arg(span: Span) -> Arg { Arg { ty: P(ty), pat: pat, id: ast::DUMMY_NODE_ID } } -type RewindPoint = (token::Token, Span, Option, Span, TokenCursor, Vec); +struct RewindPoint { + token: token::Token, + span: Span, + meta_var_span: Option, + prev_span: Span, + token_cursor: TokenCursor, + expected_tokens: Vec, +} impl<'a> Parser<'a> { pub fn new(sess: &'a ParseSess, @@ -2835,11 +2842,13 @@ impl<'a> Parser<'a> { // 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) { @@ -2850,15 +2859,17 @@ impl<'a> Parser<'a> { Ok(lstring) => format!("`{}`", lstring), _ => "a type".to_string(), }; - err.span_suggestion(suggestion_span, + 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); - err.level = Level::Warning; - err.set_message(&format!("`<` is interpreted as a start of generic \ - arguments for {}, not a comparison", - warn_message)); - err.emit(); + warn.emit(); path } Err(mut path_err) => { @@ -6255,23 +6266,22 @@ impl<'a> Parser<'a> { } fn get_rewind_point(&mut self) -> RewindPoint { - ( - self.token.clone(), - self.span, - self.meta_var_span, - self.prev_span, - self.token_cursor.clone(), - self.expected_tokens.clone(), - ) + 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) { - let (token, span, meta_var_span, prev_span, token_cursor, expected_tokens,) = rp; - self.token = token; - self.span = span; - self.meta_var_span = meta_var_span; - self.prev_span = prev_span; - self.token_cursor = token_cursor; - self.expected_tokens = expected_tokens; + 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/ui/issue-22644.stderr b/src/test/ui/issue-22644.stderr index 2cbb42653565d..1e7861755877e 100644 --- a/src/test/ui/issue-22644.stderr +++ b/src/test/ui/issue-22644.stderr @@ -2,7 +2,9 @@ warning: `<` is interpreted as a start of generic arguments for `usize`, not a c --> $DIR/issue-22644.rs:16:33 | 16 | println!("{}", a as usize < b); - | ^ expected one of `!`, `(`, `+`, `,`, `::`, or `>` here + | - ^ interpreted as generic argument + | | + | not interpreted as comparison | help: if you want to compare the casted value then write: | println!("{}", (a as usize) < b); @@ -11,9 +13,9 @@ warning: `<` is interpreted as a start of generic arguments for `usize`, not a c --> $DIR/issue-22644.rs:17:33 | 17 | println!("{}", a as usize < 4); - | -^ unexpected token - | | - | expected one of `>`, identifier, lifetime, or type here + | - ^ interpreted as generic argument + | | + | not interpreted as comparison | help: if you want to compare the casted value then write: | println!("{}", (a as usize) < 4); From ad260ffc884477686e8f6f52c97a417bf99e2f19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 14 Jun 2017 20:42:24 -0700 Subject: [PATCH 5/5] 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 --- src/libsyntax/parse/parser.rs | 170 ++++++++---------- src/test/parse-fail/better-expected.rs | 2 +- src/test/parse-fail/bounds-type-where.rs | 2 +- src/test/parse-fail/closure-return-syntax.rs | 2 +- src/test/parse-fail/empty-impl-semicolon.rs | 2 +- src/test/parse-fail/multitrait.rs | 2 +- .../removed-syntax-closure-lifetime.rs | 2 +- .../parse-fail/removed-syntax-fixed-vec.rs | 2 +- .../parse-fail/removed-syntax-ptr-lifetime.rs | 2 +- src/test/ui/issue-22644.stderr | 6 +- 10 files changed, 83 insertions(+), 109 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index b3623fbe3ac1a..3e62f6e33e09a 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -150,7 +150,7 @@ fn maybe_append(mut lhs: Vec, rhs: Option>) 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, - prev_span: Span, - token_cursor: TokenCursor, - expected_tokens: Vec, -} - 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::c` /// - `a::b::c(V) -> W` /// - `a::b::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> { 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!("()") - }; - 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, lhs_span: Span) -> PResult<'a, P> { + 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!("( as )") + }; + 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 index d78ac3cb5ce90..b60201c251dcc 100644 --- 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 index 1037854959331..9dc5d8277446d 100644 --- 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 index f3be750e25462..1da6735918012 100644 --- 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 index ef25e5d0fc2e4..9939f1e36ea9d 100644 --- 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 index d2be89d19ecee..b7c9b16588466 100644 --- 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 index 2dd2bd22d1a3d..b305b1894a810 100644 --- 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; -//~^ 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 index f943d5d50cac4..0f34db0885202 100644 --- 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 index 40f368e710543..b91ab8730b3dc 100644 --- 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 index 1e7861755877e..a22496357d991 100644 --- 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) +