From f007e6f442adafae3e5f2f7f635dc12463bbe0bb Mon Sep 17 00:00:00 2001 From: Esteban Küber Date: Mon, 22 Apr 2019 19:37:23 -0700 Subject: 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/libsyntax/parse/parser.rs | 64 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 3 deletions(-) (limited to 'src/libsyntax/parse/parser.rs') diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 8efe84cdf01..3c7f477cc8f 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); } } -- cgit 1.4.1-3-g733a5 From 617ce2b7ee330bbcc7ce8eb87160c71ad995639b Mon Sep 17 00:00:00 2001 From: Esteban Küber Date: Tue, 30 Apr 2019 20:37:42 -0700 Subject: 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(-) (limited to 'src/libsyntax/parse/parser.rs') diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 3c7f477cc8f..fbd1203dec9 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 123f06e7707..1ce6f9c2503 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 6f713c08940..b526c17488e 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 be577b8f36f..1725ba944c5 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 { -- cgit 1.4.1-3-g733a5 From e0cef5cf406016d5c1685a164a27571d182fa873 Mon Sep 17 00:00:00 2001 From: Esteban Küber Date: Thu, 2 May 2019 15:53:09 -0700 Subject: 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 ++++++------ src/test/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 +- .../ui/parser/struct-literal-in-match-discriminant.stderr | 2 +- src/test/ui/parser/struct-literal-in-while.stderr | 2 +- .../ui/parser/struct-literal-restrictions-in-lamda.stderr | 2 +- src/test/ui/struct-literal-variant-in-if.stderr | 8 ++++---- 11 files changed, 23 insertions(+), 23 deletions(-) (limited to 'src/libsyntax/parse/parser.rs') diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 61270716dfc..a0050b13596 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 fbd1203dec9..02e6c5e1c8d 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 5cb7121a0d1..ec240003f91 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 1725ba944c5..a11209998a7 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 0d7f81645b4..bb7df30783a 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 3c3f6e7f032..29af72a5d23 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 851c495abb4..e76c1cb45dd 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 0058e8981cd..95b0882b7ae 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 9959a57be85..acd31b477dc 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 81f7a91ddb3..24078074161 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 55f23baea7a..f91b9d7dce6 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: "" }) {} | ^ ^ -- cgit 1.4.1-3-g733a5 From f6a4b5270a0007e950546828a7f6fc7c54354b96 Mon Sep 17 00:00:00 2001 From: Esteban Küber Date: Thu, 2 May 2019 16:13:28 -0700 Subject: 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(-) (limited to 'src/libsyntax/parse/parser.rs') diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index a0050b13596..763d6b898a4 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 94bbd5ba2f7..0d41a1ff849 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 02e6c5e1c8d..66d45f799d9 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); } -- cgit 1.4.1-3-g733a5 From 54430ad53ab00bf86090a8d11844db1c40b2ca24 Mon Sep 17 00:00:00 2001 From: Esteban Küber Date: Mon, 6 May 2019 16:00:21 -0700 Subject: 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(-) (limited to 'src/libsyntax/parse/parser.rs') diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 763d6b898a4..cc73e1753d4 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 e7d79a647d3..4e5a51bdd9a 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 0d41a1ff849..0ad60369422 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 66d45f799d9..460d829fc06 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 d76dede8155..828fbaef985 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, } - } } -- cgit 1.4.1-3-g733a5 From dcd3cf70177f5af115d4799fc7fff836b3bcf649 Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 9 May 2019 19:10:27 +0100 Subject: Do not modify mutability of simple bindings. This commit removes the modification of the mutability of simple bindings. While the mutability isn't used, it is important that it is kept so that the input to procedural macros matches what the user wrote. This commit also modifies the span of the binding mode so that it is considered a compiler desugaring and won't be linted against for being unused.. --- src/librustc/hir/lowering.rs | 44 ++++++++---------------------- src/libsyntax/parse/parser.rs | 23 +++++++++------- src/libsyntax/source_map.rs | 21 ++++++++++++++ src/test/ui/async-await/issue-60674.stdout | 2 +- 4 files changed, 46 insertions(+), 44 deletions(-) (limited to 'src/libsyntax/parse/parser.rs') diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 20e016b8b5b..2f1bb1475bf 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -50,7 +50,6 @@ use errors::Applicability; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::indexed_vec::IndexVec; use rustc_data_structures::thin_vec::ThinVec; -use rustc_data_structures::sync::Lrc; use std::collections::{BTreeSet, BTreeMap}; use std::mem; @@ -59,10 +58,10 @@ use syntax::attr; use syntax::ast; use syntax::ast::*; use syntax::errors; -use syntax::ext::hygiene::{Mark, SyntaxContext}; +use syntax::ext::hygiene::Mark; use syntax::print::pprust; use syntax::ptr::P; -use syntax::source_map::{self, respan, CompilerDesugaringKind, Spanned}; +use syntax::source_map::{respan, CompilerDesugaringKind, Spanned}; use syntax::std_inject; use syntax::symbol::{keywords, Symbol}; use syntax::tokenstream::{TokenStream, TokenTree}; @@ -855,27 +854,6 @@ impl<'a> LoweringContext<'a> { Ident::with_empty_ctxt(Symbol::gensym(s)) } - /// Reuses the span but adds information like the kind of the desugaring and features that are - /// allowed inside this span. - fn mark_span_with_reason( - &self, - reason: CompilerDesugaringKind, - span: Span, - allow_internal_unstable: Option>, - ) -> Span { - let mark = Mark::fresh(Mark::root()); - mark.set_expn_info(source_map::ExpnInfo { - call_site: span, - def_site: Some(span), - format: source_map::CompilerDesugaring(reason), - allow_internal_unstable, - allow_internal_unsafe: false, - local_inner_macros: false, - edition: source_map::hygiene::default_edition(), - }); - span.with_ctxt(SyntaxContext::empty().apply_mark(mark)) - } - fn with_anonymous_lifetime_mode( &mut self, anonymous_lifetime_mode: AnonymousLifetimeMode, @@ -1164,7 +1142,7 @@ impl<'a> LoweringContext<'a> { attrs: ThinVec::new(), }; - let unstable_span = self.mark_span_with_reason( + let unstable_span = self.sess.source_map().mark_span_with_reason( CompilerDesugaringKind::Async, span, Some(vec![ @@ -1571,7 +1549,7 @@ impl<'a> LoweringContext<'a> { // desugaring that explicitly states that we don't want to track that. // Not tracking it makes lints in rustc and clippy very fragile as // frequently opened issues show. - let exist_ty_span = self.mark_span_with_reason( + let exist_ty_span = self.sess.source_map().mark_span_with_reason( CompilerDesugaringKind::ExistentialReturnType, span, None, @@ -2446,7 +2424,7 @@ impl<'a> LoweringContext<'a> { ) -> hir::FunctionRetTy { let span = output.span(); - let exist_ty_span = self.mark_span_with_reason( + let exist_ty_span = self.sess.source_map().mark_span_with_reason( CompilerDesugaringKind::Async, span, None, @@ -4182,7 +4160,7 @@ impl<'a> LoweringContext<'a> { }), ExprKind::TryBlock(ref body) => { self.with_catch_scope(body.id, |this| { - let unstable_span = this.mark_span_with_reason( + let unstable_span = this.sess.source_map().mark_span_with_reason( CompilerDesugaringKind::TryBlock, body.span, Some(vec![ @@ -4615,7 +4593,7 @@ impl<'a> LoweringContext<'a> { // expand let mut head = self.lower_expr(head); let head_sp = head.span; - let desugared_span = self.mark_span_with_reason( + let desugared_span = self.sess.source_map().mark_span_with_reason( CompilerDesugaringKind::ForLoop, head_sp, None, @@ -4776,7 +4754,7 @@ impl<'a> LoweringContext<'a> { // return Try::from_error(From::from(err)), // } - let unstable_span = self.mark_span_with_reason( + let unstable_span = self.sess.source_map().mark_span_with_reason( CompilerDesugaringKind::QuestionMark, e.span, Some(vec![ @@ -4784,7 +4762,7 @@ impl<'a> LoweringContext<'a> { ].into()), ); let try_span = self.sess.source_map().end_point(e.span); - let try_span = self.mark_span_with_reason( + let try_span = self.sess.source_map().mark_span_with_reason( CompilerDesugaringKind::QuestionMark, try_span, Some(vec![ @@ -5569,12 +5547,12 @@ impl<'a> LoweringContext<'a> { ); self.sess.abort_if_errors(); } - let span = self.mark_span_with_reason( + let span = self.sess.source_map().mark_span_with_reason( CompilerDesugaringKind::Await, await_span, None, ); - let gen_future_span = self.mark_span_with_reason( + let gen_future_span = self.sess.source_map().mark_span_with_reason( CompilerDesugaringKind::Await, await_span, Some(vec![Symbol::intern("gen_future")].into()), diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index f3eac71ee77..ab4de4891a5 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -50,7 +50,10 @@ use crate::symbol::{Symbol, keywords}; use errors::{Applicability, DiagnosticBuilder, DiagnosticId, FatalError}; use rustc_target::spec::abi::{self, Abi}; -use syntax_pos::{Span, MultiSpan, BytePos, FileName}; +use syntax_pos::{ + Span, MultiSpan, BytePos, FileName, + hygiene::CompilerDesugaringKind, +}; use log::{debug, trace}; use std::borrow::Cow; @@ -8741,6 +8744,15 @@ impl<'a> Parser<'a> { // statement. let (binding_mode, ident, is_simple_pattern) = match input.pat.node { PatKind::Ident(binding_mode @ BindingMode::ByValue(_), ident, _) => { + // Simple patterns like this don't have a generated argument, but they are + // moved into the closure with a statement, so any `mut` bindings on the + // argument will be unused. This binding mode can't be removed, because + // this would affect the input to procedural macros, but they can have + // their span marked as being the result of a compiler desugaring so + // that they aren't linted against. + input.pat.span = self.sess.source_map().mark_span_with_reason( + CompilerDesugaringKind::Async, span, None); + (binding_mode, ident, true) } _ => (BindingMode::ByValue(Mutability::Mutable), ident, false), @@ -8810,15 +8822,6 @@ impl<'a> Parser<'a> { }) }; - // Remove mutability from arguments. If this is not a simple pattern, - // those arguments are replaced by `__argN`, so there is no need to do this. - if let PatKind::Ident(BindingMode::ByValue(mutability @ Mutability::Mutable), ..) = - &mut input.pat.node - { - assert!(is_simple_pattern); - *mutability = Mutability::Immutable; - } - let move_stmt = Stmt { id, node: StmtKind::Local(P(move_local)), span }; arguments.push(AsyncArgument { ident, arg, pat_stmt, move_stmt }); } diff --git a/src/libsyntax/source_map.rs b/src/libsyntax/source_map.rs index 08abbf5e8a4..215618bd09c 100644 --- a/src/libsyntax/source_map.rs +++ b/src/libsyntax/source_map.rs @@ -930,6 +930,27 @@ impl SourceMap { None } + + /// Reuses the span but adds information like the kind of the desugaring and features that are + /// allowed inside this span. + pub fn mark_span_with_reason( + &self, + reason: hygiene::CompilerDesugaringKind, + span: Span, + allow_internal_unstable: Option>, + ) -> Span { + let mark = Mark::fresh(Mark::root()); + mark.set_expn_info(ExpnInfo { + call_site: span, + def_site: Some(span), + format: CompilerDesugaring(reason), + allow_internal_unstable, + allow_internal_unsafe: false, + local_inner_macros: false, + edition: hygiene::default_edition(), + }); + span.with_ctxt(SyntaxContext::empty().apply_mark(mark)) + } } impl SourceMapper for SourceMap { diff --git a/src/test/ui/async-await/issue-60674.stdout b/src/test/ui/async-await/issue-60674.stdout index f1dbcbaac0b..a93944db1c5 100644 --- a/src/test/ui/async-await/issue-60674.stdout +++ b/src/test/ui/async-await/issue-60674.stdout @@ -1 +1 @@ -async fn f(x: u8) { } +async fn f(mut x: u8) { } -- cgit 1.4.1-3-g733a5 From d5e04067cb23df91070fea1a01aa6417afa714ed Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 9 May 2019 19:14:39 +0100 Subject: Add FIXME about `construct_async_arguments`. This is unrelated to the rest of this PR but it made sense to add a FIXME explaining that the function shouldn't really be in the parser. --- src/libsyntax/parse/parser.rs | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/libsyntax/parse/parser.rs') diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ab4de4891a5..5fd6276a237 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -8730,6 +8730,10 @@ impl<'a> Parser<'a> { /// The arguments of the function are replaced in HIR lowering with the arguments created by /// this function and the statements created here are inserted at the top of the closure body. fn construct_async_arguments(&mut self, asyncness: &mut Spanned, decl: &mut FnDecl) { + // FIXME(davidtwco): This function should really live in the HIR lowering but because + // the types constructed here need to be used in parts of resolve so that the correct + // locals are considered upvars, it is currently easier for it to live here in the parser, + // where it can be constructed once. if let IsAsync::Async { ref mut arguments, .. } = asyncness.node { for (index, input) in decl.inputs.iter_mut().enumerate() { let id = ast::DUMMY_NODE_ID; -- cgit 1.4.1-3-g733a5