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 1/6] 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 f11638478923f..61270716dfcbc 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 cf8f8abe2ab50..e7d79a647d360 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 1abc7832ffa0f..94bbd5ba2f75b 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 8efe84cdf016f..3c7f477cc8f00 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 5f15ede7b0b6a..d76dede8155a0 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 0000000000000..a0abd00a15c2a --- /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 0000000000000..cf2e7266a4aac --- /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 0000000000000..0311960543277 --- /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 1a40d67492990..56c917c7462f2 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 a844cac189aa1..0d7f81645b46a 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 From bff0be37845a96010fa2161bbf137fadfe763ae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Apr 2019 14:35:09 -0700 Subject: [PATCH 2/6] Add test case for #47287 --- src/test/ui/parser/expr-as-stmt.fixed | 6 ++++++ src/test/ui/parser/expr-as-stmt.rs | 6 ++++++ src/test/ui/parser/expr-as-stmt.stderr | 14 +++++++++++++- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/test/ui/parser/expr-as-stmt.fixed b/src/test/ui/parser/expr-as-stmt.fixed index a0abd00a15c2a..123f06e7707e0 100644 --- a/src/test/ui/parser/expr-as-stmt.fixed +++ b/src/test/ui/parser/expr-as-stmt.fixed @@ -31,4 +31,10 @@ fn qux(a: Option, b: Option) -> bool { if let Some(y) = a { true } else { false } } +fn moo(x: u32) -> bool { + (match x { + _ => 1, + }) > 0 //~ ERROR ambiguous parse +} + fn main() {} diff --git a/src/test/ui/parser/expr-as-stmt.rs b/src/test/ui/parser/expr-as-stmt.rs index cf2e7266a4aac..6f713c0894001 100644 --- a/src/test/ui/parser/expr-as-stmt.rs +++ b/src/test/ui/parser/expr-as-stmt.rs @@ -31,4 +31,10 @@ fn qux(a: Option, b: Option) -> bool { if let Some(y) = a { true } else { false } } +fn moo(x: u32) -> bool { + match x { + _ => 1, + } > 0 //~ ERROR ambiguous parse +} + fn main() {} diff --git a/src/test/ui/parser/expr-as-stmt.stderr b/src/test/ui/parser/expr-as-stmt.stderr index 0311960543277..be577b8f36fe6 100644 --- a/src/test/ui/parser/expr-as-stmt.stderr +++ b/src/test/ui/parser/expr-as-stmt.stderr @@ -30,6 +30,18 @@ LL | if let Some(x) = a { true } else { false } LL | && | ^^ +error: ambiguous parse + --> $DIR/expr-as-stmt.rs:37:7 + | +LL | } > 0 + | ^ +help: parenthesis are required to parse this as an expression + | +LL | (match x { +LL | _ => 1, +LL | }) > 0 + | + error[E0308]: mismatched types --> $DIR/expr-as-stmt.rs:7:6 | @@ -74,7 +86,7 @@ LL | { 3 } * 3 | | | help: parenthesis are required to parse this as an expression: `({ 3 })` -error: aborting due to 9 previous errors +error: aborting due to 10 previous errors Some errors have detailed explanations: E0308, E0614. For more information about an error, try `rustc --explain E0308`. From 617ce2b7ee330bbcc7ce8eb87160c71ad995639b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 30 Apr 2019 20:37:42 -0700 Subject: [PATCH 3/6] Reword ambigous parse error to fit with the current error --- src/libsyntax/parse/parser.rs | 11 ++++++----- src/test/ui/parser/expr-as-stmt.fixed | 4 ++-- src/test/ui/parser/expr-as-stmt.rs | 4 ++-- src/test/ui/parser/expr-as-stmt.stderr | 8 ++++---- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 3c7f477cc8f00..fbd1203dec94a 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3649,13 +3649,14 @@ impl<'a> Parser<'a> { 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", - ); + // For example: `if let Some(x) = x { x } else { 0 } / 2` + let mut err = self.sess.span_diagnostic.struct_span_err(self.span, &format!( + "expected expression, found `{}`", + pprust::token_to_string(&self.token), + )); + err.span_label(self.span, "expected expression"); let snippet = self.sess.source_map().span_to_snippet(lhs.span) .unwrap_or_else(|_| pprust::expr_to_string(&lhs)); err.span_suggestion( diff --git a/src/test/ui/parser/expr-as-stmt.fixed b/src/test/ui/parser/expr-as-stmt.fixed index 123f06e7707e0..1ce6f9c25034f 100644 --- a/src/test/ui/parser/expr-as-stmt.fixed +++ b/src/test/ui/parser/expr-as-stmt.fixed @@ -27,14 +27,14 @@ fn baz() -> i32 { fn qux(a: Option, b: Option) -> bool { (if let Some(x) = a { true } else { false }) - && //~ ERROR ambiguous parse + && //~ ERROR expected expression if let Some(y) = a { true } else { false } } fn moo(x: u32) -> bool { (match x { _ => 1, - }) > 0 //~ ERROR ambiguous parse + }) > 0 //~ ERROR expected expression } fn main() {} diff --git a/src/test/ui/parser/expr-as-stmt.rs b/src/test/ui/parser/expr-as-stmt.rs index 6f713c0894001..b526c17488eaf 100644 --- a/src/test/ui/parser/expr-as-stmt.rs +++ b/src/test/ui/parser/expr-as-stmt.rs @@ -27,14 +27,14 @@ fn baz() -> i32 { fn qux(a: Option, b: Option) -> bool { if let Some(x) = a { true } else { false } - && //~ ERROR ambiguous parse + && //~ ERROR expected expression if let Some(y) = a { true } else { false } } fn moo(x: u32) -> bool { match x { _ => 1, - } > 0 //~ ERROR ambiguous parse + } > 0 //~ ERROR expected expression } fn main() {} diff --git a/src/test/ui/parser/expr-as-stmt.stderr b/src/test/ui/parser/expr-as-stmt.stderr index be577b8f36fe6..1725ba944c50f 100644 --- a/src/test/ui/parser/expr-as-stmt.stderr +++ b/src/test/ui/parser/expr-as-stmt.stderr @@ -22,19 +22,19 @@ LL | { 42 } + foo; | | | help: parenthesis are required to parse this as an expression: `({ 42 })` -error: ambiguous parse +error: expected expression, found `&&` --> $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 | && - | ^^ + | ^^ expected expression -error: ambiguous parse +error: expected expression, found `>` --> $DIR/expr-as-stmt.rs:37:7 | LL | } > 0 - | ^ + | ^ expected expression help: parenthesis are required to parse this as an expression | LL | (match x { From e0cef5cf406016d5c1685a164a27571d182fa873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 2 May 2019 15:53:09 -0700 Subject: [PATCH 4/6] fix typo --- src/librustc_typeck/check/mod.rs | 2 +- src/libsyntax/parse/parser.rs | 8 ++++---- src/test/ui/error-codes/E0423.stderr | 4 ++-- src/test/ui/parser/expr-as-stmt.stderr | 12 ++++++------ .../ui/parser/match-arrows-block-then-binop.stderr | 2 +- src/test/ui/parser/struct-literal-in-for.stderr | 2 +- src/test/ui/parser/struct-literal-in-if.stderr | 2 +- .../struct-literal-in-match-discriminant.stderr | 2 +- src/test/ui/parser/struct-literal-in-while.stderr | 2 +- .../struct-literal-restrictions-in-lamda.stderr | 2 +- src/test/ui/struct-literal-variant-in-if.stderr | 8 ++++---- 11 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 61270716dfcbc..a0050b1359679 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4182,7 +4182,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { { err.span_suggestion( *sp, - "parenthesis are required to parse this \ + "parentheses are required to parse this \ as an expression", format!("({})", snippet), Applicability::MachineApplicable, diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index fbd1203dec94a..02e6c5e1c8dbf 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2934,7 +2934,7 @@ impl<'a> Parser<'a> { 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", + "parentheses are required to parse this as an expression", format!("({})", snippet), Applicability::MachineApplicable, ); @@ -2979,7 +2979,7 @@ impl<'a> Parser<'a> { "struct literals are not allowed here", ); err.multipart_suggestion( - "surround the struct literal with parenthesis", + "surround the struct literal with parentheses", vec![ (lo.shrink_to_lo(), "(".to_string()), (expr.span.shrink_to_hi(), ")".to_string()), @@ -3661,7 +3661,7 @@ impl<'a> Parser<'a> { .unwrap_or_else(|_| pprust::expr_to_string(&lhs)); err.span_suggestion( lhs.span, - "parenthesis are required to parse this as an expression", + "parentheses are required to parse this as an expression", format!("({})", snippet), Applicability::MachineApplicable, ); @@ -4982,7 +4982,7 @@ impl<'a> Parser<'a> { 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", + "parentheses are required to parse this as an expression", format!("({})", snippet), Applicability::MachineApplicable, ); diff --git a/src/test/ui/error-codes/E0423.stderr b/src/test/ui/error-codes/E0423.stderr index 5cb7121a0d1c3..ec240003f9182 100644 --- a/src/test/ui/error-codes/E0423.stderr +++ b/src/test/ui/error-codes/E0423.stderr @@ -3,7 +3,7 @@ error: struct literals are not allowed here | LL | if let S { x: _x, y: 2 } = S { x: 1, y: 2 } { println!("Ok"); } | ^^^^^^^^^^^^^^^^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | if let S { x: _x, y: 2 } = (S { x: 1, y: 2 }) { println!("Ok"); } | ^ ^ @@ -19,7 +19,7 @@ error: struct literals are not allowed here | LL | for _ in std::ops::Range { start: 0, end: 10 } {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | for _ in (std::ops::Range { start: 0, end: 10 }) {} | ^ ^ diff --git a/src/test/ui/parser/expr-as-stmt.stderr b/src/test/ui/parser/expr-as-stmt.stderr index 1725ba944c50f..a11209998a7d5 100644 --- a/src/test/ui/parser/expr-as-stmt.stderr +++ b/src/test/ui/parser/expr-as-stmt.stderr @@ -4,7 +4,7 @@ error: expected expression, found `+` LL | {2} + {2} | --- ^ expected expression | | - | help: parenthesis are required to parse this as an expression: `({2})` + | help: parentheses are required to parse this as an expression: `({2})` error: expected expression, found `+` --> $DIR/expr-as-stmt.rs:12:9 @@ -12,7 +12,7 @@ error: expected expression, found `+` LL | {2} + 2 | --- ^ expected expression | | - | help: parenthesis are required to parse this as an expression: `({2})` + | help: parentheses are required to parse this as an expression: `({2})` error: expected expression, found `+` --> $DIR/expr-as-stmt.rs:18:12 @@ -20,13 +20,13 @@ error: expected expression, found `+` LL | { 42 } + foo; | ------ ^ expected expression | | - | help: parenthesis are required to parse this as an expression: `({ 42 })` + | help: parentheses are required to parse this as an expression: `({ 42 })` error: expected expression, found `&&` --> $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 })` + | ------------------------------------------ help: parentheses are required to parse this as an expression: `(if let Some(x) = a { true } else { false })` LL | && | ^^ expected expression @@ -35,7 +35,7 @@ error: expected expression, found `>` | LL | } > 0 | ^ expected expression -help: parenthesis are required to parse this as an expression +help: parentheses are required to parse this as an expression | LL | (match x { LL | _ => 1, @@ -84,7 +84,7 @@ error[E0614]: type `{integer}` cannot be dereferenced LL | { 3 } * 3 | ----- ^^^ | | - | help: parenthesis are required to parse this as an expression: `({ 3 })` + | help: parentheses are required to parse this as an expression: `({ 3 })` error: aborting due to 10 previous errors 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 0d7f81645b46a..bb7df30783acd 100644 --- a/src/test/ui/parser/match-arrows-block-then-binop.stderr +++ b/src/test/ui/parser/match-arrows-block-then-binop.stderr @@ -3,7 +3,7 @@ error: expected pattern, found `+` | LL | } + 5 | ^ expected pattern -help: parenthesis are required to parse this as an expression +help: parentheses are required to parse this as an expression | LL | 0 => ({ LL | 0 diff --git a/src/test/ui/parser/struct-literal-in-for.stderr b/src/test/ui/parser/struct-literal-in-for.stderr index 3c3f6e7f032f6..29af72a5d23d0 100644 --- a/src/test/ui/parser/struct-literal-in-for.stderr +++ b/src/test/ui/parser/struct-literal-in-for.stderr @@ -6,7 +6,7 @@ LL | for x in Foo { LL | | x: 3 LL | | }.hi() { | |_____^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | for x in (Foo { LL | x: 3 diff --git a/src/test/ui/parser/struct-literal-in-if.stderr b/src/test/ui/parser/struct-literal-in-if.stderr index 851c495abb4b0..e76c1cb45dd4e 100644 --- a/src/test/ui/parser/struct-literal-in-if.stderr +++ b/src/test/ui/parser/struct-literal-in-if.stderr @@ -6,7 +6,7 @@ LL | if Foo { LL | | x: 3 LL | | }.hi() { | |_____^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | if (Foo { LL | x: 3 diff --git a/src/test/ui/parser/struct-literal-in-match-discriminant.stderr b/src/test/ui/parser/struct-literal-in-match-discriminant.stderr index 0058e8981cd25..95b0882b7aeb5 100644 --- a/src/test/ui/parser/struct-literal-in-match-discriminant.stderr +++ b/src/test/ui/parser/struct-literal-in-match-discriminant.stderr @@ -6,7 +6,7 @@ LL | match Foo { LL | | x: 3 LL | | } { | |_____^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | match (Foo { LL | x: 3 diff --git a/src/test/ui/parser/struct-literal-in-while.stderr b/src/test/ui/parser/struct-literal-in-while.stderr index 9959a57be8596..acd31b477dc27 100644 --- a/src/test/ui/parser/struct-literal-in-while.stderr +++ b/src/test/ui/parser/struct-literal-in-while.stderr @@ -6,7 +6,7 @@ LL | while Foo { LL | | x: 3 LL | | }.hi() { | |_____^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | while (Foo { LL | x: 3 diff --git a/src/test/ui/parser/struct-literal-restrictions-in-lamda.stderr b/src/test/ui/parser/struct-literal-restrictions-in-lamda.stderr index 81f7a91ddb38a..24078074161e6 100644 --- a/src/test/ui/parser/struct-literal-restrictions-in-lamda.stderr +++ b/src/test/ui/parser/struct-literal-restrictions-in-lamda.stderr @@ -6,7 +6,7 @@ LL | while || Foo { LL | | x: 3 LL | | }.hi() { | |_____^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | while || (Foo { LL | x: 3 diff --git a/src/test/ui/struct-literal-variant-in-if.stderr b/src/test/ui/struct-literal-variant-in-if.stderr index 55f23baea7aa8..f91b9d7dce60f 100644 --- a/src/test/ui/struct-literal-variant-in-if.stderr +++ b/src/test/ui/struct-literal-variant-in-if.stderr @@ -3,7 +3,7 @@ error: struct literals are not allowed here | LL | if x == E::I { field1: true, field2: 42 } {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | if x == (E::I { field1: true, field2: 42 }) {} | ^ ^ @@ -13,7 +13,7 @@ error: struct literals are not allowed here | LL | if x == E::V { field: false } {} | ^^^^^^^^^^^^^^^^^^^^^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | if x == (E::V { field: false }) {} | ^ ^ @@ -23,7 +23,7 @@ error: struct literals are not allowed here | LL | if x == E::J { field: -42 } {} | ^^^^^^^^^^^^^^^^^^^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | if x == (E::J { field: -42 }) {} | ^ ^ @@ -33,7 +33,7 @@ error: struct literals are not allowed here | LL | if x == E::K { field: "" } {} | ^^^^^^^^^^^^^^^^^^ -help: surround the struct literal with parenthesis +help: surround the struct literal with parentheses | LL | if x == (E::K { field: "" }) {} | ^ ^ From f6a4b5270a0007e950546828a7f6fc7c54354b96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 2 May 2019 16:13:28 -0700 Subject: [PATCH 5/6] Deduplicate needed parentheses suggestion code --- src/librustc_typeck/check/mod.rs | 16 +++++----------- src/libsyntax/parse/mod.rs | 23 ++++++++++++++++++++++- src/libsyntax/parse/parser.rs | 29 ++++++----------------------- 3 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index a0050b1359679..763d6b898a484 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4177,17 +4177,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { 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, - "parentheses are required to parse this \ - as an expression", - format!("({})", snippet), - Applicability::MachineApplicable, - ); - } + tcx.sess.parse_sess.expr_parentheses_needed( + &mut err, + *sp, + None, + ); } err.emit(); oprnd_t = tcx.types.err; diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 94bbd5ba2f75b..0d41a1ff84904 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -11,7 +11,7 @@ use crate::tokenstream::{TokenStream, TokenTree}; use crate::diagnostics::plugin::ErrorMap; use crate::print::pprust::token_to_string; -use errors::{FatalError, Level, Handler, ColorConfig, Diagnostic, DiagnosticBuilder}; +use errors::{Applicability, FatalError, Level, Handler, ColorConfig, Diagnostic, DiagnosticBuilder}; use rustc_data_structures::sync::{Lrc, Lock}; use syntax_pos::{Span, SourceFile, FileName, MultiSpan}; use log::debug; @@ -47,6 +47,9 @@ pub struct ParseSess { included_mod_stack: Lock>, source_map: Lrc, pub buffered_lints: Lock>, + /// Contains the spans of block expressions that could have been incomplete based on the + /// operation token that followed it, but that the parser cannot identify without further + /// analysis. pub abiguous_block_expr_parse: Lock>, } @@ -95,6 +98,24 @@ impl ParseSess { }); }); } + + /// Extend an error with a suggestion to wrap an expression with parentheses to allow the + /// parser to continue parsing the following operation as part of the same expression. + pub fn expr_parentheses_needed( + &self, + err: &mut DiagnosticBuilder<'_>, + span: Span, + alt_snippet: Option, + ) { + if let Some(snippet) = self.source_map().span_to_snippet(span).ok().or(alt_snippet) { + err.span_suggestion( + span, + "parentheses are required to parse this as an expression", + format!("({})", snippet), + Applicability::MachineApplicable, + ); + } + } } #[derive(Clone)] diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 02e6c5e1c8dbf..66d45f799d97d 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2931,14 +2931,7 @@ impl<'a> Parser<'a> { 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, - "parentheses are required to parse this as an expression", - format!("({})", snippet), - Applicability::MachineApplicable, - ); - } + self.sess.expr_parentheses_needed(&mut err, *sp, None); } err.span_label(self.span, "expected expression"); return Err(err); @@ -3657,14 +3650,11 @@ impl<'a> Parser<'a> { pprust::token_to_string(&self.token), )); err.span_label(self.span, "expected expression"); - let snippet = self.sess.source_map().span_to_snippet(lhs.span) - .unwrap_or_else(|_| pprust::expr_to_string(&lhs)); - err.span_suggestion( + self.sess.expr_parentheses_needed( + &mut err, lhs.span, - "parentheses are required to parse this as an expression", - format!("({})", snippet), - Applicability::MachineApplicable, - ); + Some(pprust::expr_to_string(&lhs), + )); err.emit(); } } @@ -4979,14 +4969,7 @@ impl<'a> Parser<'a> { 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, - "parentheses are required to parse this as an expression", - format!("({})", snippet), - Applicability::MachineApplicable, - ); - } + self.sess.expr_parentheses_needed(&mut err, *sp, None); } return Err(err); } From 54430ad53ab00bf86090a8d11844db1c40b2ca24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 6 May 2019 16:00:21 -0700 Subject: [PATCH 6/6] review comments: fix typo and add comments --- src/librustc_typeck/check/mod.rs | 2 +- src/libsyntax/parse/lexer/mod.rs | 2 +- src/libsyntax/parse/mod.rs | 4 ++-- src/libsyntax/parse/parser.rs | 11 +++++++---- src/libsyntax/util/parser.rs | 5 ++++- 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 763d6b898a484..cc73e1753d470 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4174,7 +4174,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { oprnd_t, ); let sp = tcx.sess.source_map().start_point(expr.span); - if let Some(sp) = tcx.sess.parse_sess.abiguous_block_expr_parse + if let Some(sp) = tcx.sess.parse_sess.ambiguous_block_expr_parse .borrow().get(&sp) { tcx.sess.parse_sess.expr_parentheses_needed( diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index e7d79a647d360..4e5a51bdd9afa 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -1918,7 +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()), + ambiguous_block_expr_parse: Lock::new(FxHashMap::default()), } } diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 0d41a1ff84904..0ad6036942280 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -50,7 +50,7 @@ pub struct ParseSess { /// Contains the spans of block expressions that could have been incomplete based on the /// operation token that followed it, but that the parser cannot identify without further /// analysis. - pub abiguous_block_expr_parse: Lock>, + pub ambiguous_block_expr_parse: Lock>, } impl ParseSess { @@ -74,7 +74,7 @@ impl ParseSess { included_mod_stack: Lock::new(vec![]), source_map, buffered_lints: Lock::new(vec![]), - abiguous_block_expr_parse: Lock::new(FxHashMap::default()), + ambiguous_block_expr_parse: Lock::new(FxHashMap::default()), } } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 66d45f799d97d..460d829fc0676 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2928,7 +2928,7 @@ impl<'a> Parser<'a> { 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() + if let Some(sp) = self.sess.ambiguous_block_expr_parse.borrow() .get(&sp) { self.sess.expr_parentheses_needed(&mut err, *sp, None); @@ -3630,12 +3630,15 @@ impl<'a> Parser<'a> { return Ok(lhs); } (false, _) => {} // continue parsing the expression - (true, Some(AssocOp::Multiply)) | // `{ 42 } *foo = bar;` + // An exhaustive check is done in the following block, but these are checked first + // because they *are* ambiguous but also reasonable looking incorrect syntax, so we + // want to keep their span info to improve diagnostics in these cases in a later stage. + (true, Some(AssocOp::Multiply)) | // `{ 42 } *foo = bar;` or `{ 42 } * 3` (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); + self.sess.ambiguous_block_expr_parse.borrow_mut().insert(sp, lhs.span); return Ok(lhs); } (true, Some(ref op)) if !op.can_continue_expr_unambiguously() => { @@ -4968,7 +4971,7 @@ 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 Some(sp) = self.sess.ambiguous_block_expr_parse.borrow().get(&sp) { self.sess.expr_parentheses_needed(&mut err, *sp, None); } return Err(err); diff --git a/src/libsyntax/util/parser.rs b/src/libsyntax/util/parser.rs index d76dede8155a0..828fbaef98540 100644 --- a/src/libsyntax/util/parser.rs +++ b/src/libsyntax/util/parser.rs @@ -208,6 +208,10 @@ impl AssocOp { } } + /// This operator could be used to follow a block unambiguously. + /// + /// This is used for error recovery at the moment, providing a suggestion to wrap blocks with + /// parentheses while having a high degree of confidence on the correctness of the suggestion. pub fn can_continue_expr_unambiguously(&self) -> bool { use AssocOp::*; match self { @@ -227,7 +231,6 @@ impl AssocOp { Colon => true, // `{ 42 }: usize` _ => false, } - } }