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.
rust-lang · bors · May 10, 2019 · Apr 23, 2019 · Apr 29, 2019 · May 1, 2019
commit f007e6f442adafae3e5f2f7f635dc12463bbe0bb
diff --git 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
@@ -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<SourceMap>) -> 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
@@ -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<Vec<PathBuf>>,
     source_map: Lrc<SourceMap>,
     pub buffered_lints: Lock<Vec<BufferedEarlyLint>>,
+    pub abiguous_block_expr_parse: Lock<FxHashMap<Span, Span>>,
 }
 
 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
@@ -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
@@ -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
@@ -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<u32>, b: Option<u32>) -> 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
@@ -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<u32>, b: Option<u32>) -> 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
@@ -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
@@ -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
@@ -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