From f007e6f442adafae3e5f2f7f635dc12463bbe0bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 22 Apr 2019 19:37:23 -0700 Subject: [PATCH] Identify when a stmt could have been parsed as an expr There are some expressions that can be parsed as a statement without a trailing semicolon depending on the context, which can lead to confusing errors due to the same looking code being accepted in some places and not others. Identify these cases and suggest enclosing in parenthesis making the parse non-ambiguous without changing the accepted grammar. --- src/librustc_typeck/check/mod.rs | 28 ++++++- src/libsyntax/parse/lexer/mod.rs | 3 +- src/libsyntax/parse/mod.rs | 4 +- src/libsyntax/parse/parser.rs | 64 ++++++++++++++- src/libsyntax/util/parser.rs | 22 +++++ src/test/ui/parser/expr-as-stmt.fixed | 34 ++++++++ src/test/ui/parser/expr-as-stmt.rs | 34 ++++++++ src/test/ui/parser/expr-as-stmt.stderr | 80 +++++++++++++++++++ .../parser/match-arrows-block-then-binop.rs | 6 +- .../match-arrows-block-then-binop.stderr | 6 ++ 10 files changed, 270 insertions(+), 11 deletions(-) create mode 100644 src/test/ui/parser/expr-as-stmt.fixed create mode 100644 src/test/ui/parser/expr-as-stmt.rs create mode 100644 src/test/ui/parser/expr-as-stmt.stderr diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index f11638478923..61270716dfcb 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4165,9 +4165,31 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { oprnd_t = self.make_overloaded_place_return_type(method).ty; self.write_method_call(expr.hir_id, method); } else { - type_error_struct!(tcx.sess, expr.span, oprnd_t, E0614, - "type `{}` cannot be dereferenced", - oprnd_t).emit(); + let mut err = type_error_struct!( + tcx.sess, + expr.span, + oprnd_t, + E0614, + "type `{}` cannot be dereferenced", + oprnd_t, + ); + let sp = tcx.sess.source_map().start_point(expr.span); + if let Some(sp) = tcx.sess.parse_sess.abiguous_block_expr_parse + .borrow().get(&sp) + { + if let Ok(snippet) = tcx.sess.source_map() + .span_to_snippet(*sp) + { + err.span_suggestion( + *sp, + "parenthesis are required to parse this \ + as an expression", + format!("({})", snippet), + Applicability::MachineApplicable, + ); + } + } + err.emit(); oprnd_t = tcx.types.err; } } diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index cf8f8abe2ab5..e7d79a647d36 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -1899,7 +1899,7 @@ mod tests { use std::io; use std::path::PathBuf; use syntax_pos::{BytePos, Span, NO_EXPANSION}; - use rustc_data_structures::fx::FxHashSet; + use rustc_data_structures::fx::{FxHashSet, FxHashMap}; use rustc_data_structures::sync::Lock; fn mk_sess(sm: Lrc) -> ParseSess { @@ -1918,6 +1918,7 @@ mod tests { raw_identifier_spans: Lock::new(Vec::new()), registered_diagnostics: Lock::new(ErrorMap::new()), buffered_lints: Lock::new(vec![]), + abiguous_block_expr_parse: Lock::new(FxHashMap::default()), } } diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 1abc7832ffa0..94bbd5ba2f75 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -16,7 +16,7 @@ use rustc_data_structures::sync::{Lrc, Lock}; use syntax_pos::{Span, SourceFile, FileName, MultiSpan}; use log::debug; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashSet, FxHashMap}; use std::borrow::Cow; use std::iter; use std::path::{Path, PathBuf}; @@ -47,6 +47,7 @@ pub struct ParseSess { included_mod_stack: Lock>, source_map: Lrc, pub buffered_lints: Lock>, + pub abiguous_block_expr_parse: Lock>, } impl ParseSess { @@ -70,6 +71,7 @@ impl ParseSess { included_mod_stack: Lock::new(vec![]), source_map, buffered_lints: Lock::new(vec![]), + abiguous_block_expr_parse: Lock::new(FxHashMap::default()), } } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 8efe84cdf016..3c7f477cc8f0 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -186,6 +186,7 @@ enum PrevTokenKind { Interpolated, Eof, Ident, + BitOr, Other, } @@ -1410,6 +1411,7 @@ impl<'a> Parser<'a> { token::DocComment(..) => PrevTokenKind::DocComment, token::Comma => PrevTokenKind::Comma, token::BinOp(token::Plus) => PrevTokenKind::Plus, + token::BinOp(token::Or) => PrevTokenKind::BitOr, token::Interpolated(..) => PrevTokenKind::Interpolated, token::Eof => PrevTokenKind::Eof, token::Ident(..) => PrevTokenKind::Ident, @@ -2925,6 +2927,19 @@ impl<'a> Parser<'a> { let msg = format!("expected expression, found {}", self.this_token_descr()); let mut err = self.fatal(&msg); + let sp = self.sess.source_map().start_point(self.span); + if let Some(sp) = self.sess.abiguous_block_expr_parse.borrow() + .get(&sp) + { + if let Ok(snippet) = self.sess.source_map().span_to_snippet(*sp) { + err.span_suggestion( + *sp, + "parenthesis are required to parse this as an expression", + format!("({})", snippet), + Applicability::MachineApplicable, + ); + } + } err.span_label(self.span, "expected expression"); return Err(err); } @@ -3616,9 +3631,41 @@ impl<'a> Parser<'a> { } }; - if self.expr_is_complete(&lhs) { - // Semi-statement forms are odd. See https://github.com/rust-lang/rust/issues/29071 - return Ok(lhs); + match (self.expr_is_complete(&lhs), AssocOp::from_token(&self.token)) { + (true, None) => { + // Semi-statement forms are odd. See https://github.com/rust-lang/rust/issues/29071 + return Ok(lhs); + } + (false, _) => {} // continue parsing the expression + (true, Some(AssocOp::Multiply)) | // `{ 42 } *foo = bar;` + (true, Some(AssocOp::Subtract)) | // `{ 42 } -5` + (true, Some(AssocOp::Add)) => { // `{ 42 } + 42 + // These cases are ambiguous and can't be identified in the parser alone + let sp = self.sess.source_map().start_point(self.span); + self.sess.abiguous_block_expr_parse.borrow_mut().insert(sp, lhs.span); + return Ok(lhs); + } + (true, Some(ref op)) if !op.can_continue_expr_unambiguously() => { + return Ok(lhs); + } + (true, Some(_)) => { + // #54186, #54482, #59975 + // We've found an expression that would be parsed as a statement, but the next + // token implies this should be parsed as an expression. + let mut err = self.sess.span_diagnostic.struct_span_err( + self.span, + "ambiguous parse", + ); + let snippet = self.sess.source_map().span_to_snippet(lhs.span) + .unwrap_or_else(|_| pprust::expr_to_string(&lhs)); + err.span_suggestion( + lhs.span, + "parenthesis are required to parse this as an expression", + format!("({})", snippet), + Applicability::MachineApplicable, + ); + err.emit(); + } } self.expected_tokens.push(TokenType::Operator); while let Some(op) = AssocOp::from_token(&self.token) { @@ -4929,6 +4976,17 @@ impl<'a> Parser<'a> { ); let mut err = self.fatal(&msg); err.span_label(self.span, format!("expected {}", expected)); + let sp = self.sess.source_map().start_point(self.span); + if let Some(sp) = self.sess.abiguous_block_expr_parse.borrow().get(&sp) { + if let Ok(snippet) = self.sess.source_map().span_to_snippet(*sp) { + err.span_suggestion( + *sp, + "parenthesis are required to parse this as an expression", + format!("({})", snippet), + Applicability::MachineApplicable, + ); + } + } return Err(err); } } diff --git a/src/libsyntax/util/parser.rs b/src/libsyntax/util/parser.rs index 5f15ede7b0b6..d76dede8155a 100644 --- a/src/libsyntax/util/parser.rs +++ b/src/libsyntax/util/parser.rs @@ -207,6 +207,28 @@ impl AssocOp { ObsoleteInPlace | Assign | AssignOp(_) | As | DotDot | DotDotEq | Colon => None } } + + pub fn can_continue_expr_unambiguously(&self) -> bool { + use AssocOp::*; + match self { + BitXor | // `{ 42 } ^ 3` + Assign | // `{ 42 } = { 42 }` + Divide | // `{ 42 } / 42` + Modulus | // `{ 42 } % 2` + ShiftRight | // `{ 42 } >> 2` + LessEqual | // `{ 42 } <= 3` + Greater | // `{ 42 } > 3` + GreaterEqual | // `{ 42 } >= 3` + AssignOp(_) | // `{ 42 } +=` + LAnd | // `{ 42 } &&foo` + As | // `{ 42 } as usize` + // Equal | // `{ 42 } == { 42 }` Accepting these here would regress incorrect + // NotEqual | // `{ 42 } != { 42 } struct literals parser recovery. + Colon => true, // `{ 42 }: usize` + _ => false, + } + + } } pub const PREC_RESET: i8 = -100; diff --git a/src/test/ui/parser/expr-as-stmt.fixed b/src/test/ui/parser/expr-as-stmt.fixed new file mode 100644 index 000000000000..a0abd00a15c2 --- /dev/null +++ b/src/test/ui/parser/expr-as-stmt.fixed @@ -0,0 +1,34 @@ +// run-rustfix +#![allow(unused_variables)] +#![allow(dead_code)] +#![allow(unused_must_use)] + +fn foo() -> i32 { + ({2}) + {2} //~ ERROR expected expression, found `+` + //~^ ERROR mismatched types +} + +fn bar() -> i32 { + ({2}) + 2 //~ ERROR expected expression, found `+` + //~^ ERROR mismatched types +} + +fn zul() -> u32 { + let foo = 3; + ({ 42 }) + foo; //~ ERROR expected expression, found `+` + //~^ ERROR mismatched types + 32 +} + +fn baz() -> i32 { + ({ 3 }) * 3 //~ ERROR type `{integer}` cannot be dereferenced + //~^ ERROR mismatched types +} + +fn qux(a: Option, b: Option) -> bool { + (if let Some(x) = a { true } else { false }) + && //~ ERROR ambiguous parse + if let Some(y) = a { true } else { false } +} + +fn main() {} diff --git a/src/test/ui/parser/expr-as-stmt.rs b/src/test/ui/parser/expr-as-stmt.rs new file mode 100644 index 000000000000..cf2e7266a4aa --- /dev/null +++ b/src/test/ui/parser/expr-as-stmt.rs @@ -0,0 +1,34 @@ +// run-rustfix +#![allow(unused_variables)] +#![allow(dead_code)] +#![allow(unused_must_use)] + +fn foo() -> i32 { + {2} + {2} //~ ERROR expected expression, found `+` + //~^ ERROR mismatched types +} + +fn bar() -> i32 { + {2} + 2 //~ ERROR expected expression, found `+` + //~^ ERROR mismatched types +} + +fn zul() -> u32 { + let foo = 3; + { 42 } + foo; //~ ERROR expected expression, found `+` + //~^ ERROR mismatched types + 32 +} + +fn baz() -> i32 { + { 3 } * 3 //~ ERROR type `{integer}` cannot be dereferenced + //~^ ERROR mismatched types +} + +fn qux(a: Option, b: Option) -> bool { + if let Some(x) = a { true } else { false } + && //~ ERROR ambiguous parse + if let Some(y) = a { true } else { false } +} + +fn main() {} diff --git a/src/test/ui/parser/expr-as-stmt.stderr b/src/test/ui/parser/expr-as-stmt.stderr new file mode 100644 index 000000000000..031196054327 --- /dev/null +++ b/src/test/ui/parser/expr-as-stmt.stderr @@ -0,0 +1,80 @@ +error: expected expression, found `+` + --> $DIR/expr-as-stmt.rs:7:9 + | +LL | {2} + {2} + | --- ^ expected expression + | | + | help: parenthesis are required to parse this as an expression: `({2})` + +error: expected expression, found `+` + --> $DIR/expr-as-stmt.rs:12:9 + | +LL | {2} + 2 + | --- ^ expected expression + | | + | help: parenthesis are required to parse this as an expression: `({2})` + +error: expected expression, found `+` + --> $DIR/expr-as-stmt.rs:18:12 + | +LL | { 42 } + foo; + | ------ ^ expected expression + | | + | help: parenthesis are required to parse this as an expression: `({ 42 })` + +error: ambiguous parse + --> $DIR/expr-as-stmt.rs:30:5 + | +LL | if let Some(x) = a { true } else { false } + | ------------------------------------------ help: parenthesis are required to parse this as an expression: `(if let Some(x) = a { true } else { false })` +LL | && + | ^^ + +error[E0308]: mismatched types + --> $DIR/expr-as-stmt.rs:7:6 + | +LL | {2} + {2} + | ^ expected (), found integer + | + = note: expected type `()` + found type `{integer}` + +error[E0308]: mismatched types + --> $DIR/expr-as-stmt.rs:12:6 + | +LL | {2} + 2 + | ^ expected (), found integer + | + = note: expected type `()` + found type `{integer}` + +error[E0308]: mismatched types + --> $DIR/expr-as-stmt.rs:18:7 + | +LL | { 42 } + foo; + | ^^ expected (), found integer + | + = note: expected type `()` + found type `{integer}` + +error[E0308]: mismatched types + --> $DIR/expr-as-stmt.rs:24:7 + | +LL | { 3 } * 3 + | ^ expected (), found integer + | + = note: expected type `()` + found type `{integer}` + +error[E0614]: type `{integer}` cannot be dereferenced + --> $DIR/expr-as-stmt.rs:24:11 + | +LL | { 3 } * 3 + | ----- ^^^ + | | + | help: parenthesis are required to parse this as an expression: `({ 3 })` + +error: aborting due to 9 previous errors + +Some errors have detailed explanations: E0308, E0614. +For more information about an error, try `rustc --explain E0308`. diff --git a/src/test/ui/parser/match-arrows-block-then-binop.rs b/src/test/ui/parser/match-arrows-block-then-binop.rs index 1a40d6749299..56c917c7462f 100644 --- a/src/test/ui/parser/match-arrows-block-then-binop.rs +++ b/src/test/ui/parser/match-arrows-block-then-binop.rs @@ -1,7 +1,7 @@ fn main() { - - match 0 { + let _ = match 0 { 0 => { + 0 } + 5 //~ ERROR expected pattern, found `+` - } + }; } diff --git a/src/test/ui/parser/match-arrows-block-then-binop.stderr b/src/test/ui/parser/match-arrows-block-then-binop.stderr index a844cac189aa..0d7f81645b46 100644 --- a/src/test/ui/parser/match-arrows-block-then-binop.stderr +++ b/src/test/ui/parser/match-arrows-block-then-binop.stderr @@ -3,6 +3,12 @@ error: expected pattern, found `+` | LL | } + 5 | ^ expected pattern +help: parenthesis are required to parse this as an expression + | +LL | 0 => ({ +LL | 0 +LL | }) + 5 + | error: aborting due to previous error