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/lexer/mod.rs | 3 +- src/libsyntax/parse/mod.rs | 4 ++- src/libsyntax/parse/parser.rs | 64 ++++++++++++++++++++++++++++++++++++++-- src/libsyntax/util/parser.rs | 22 ++++++++++++++ 4 files changed, 88 insertions(+), 5 deletions(-) (limited to 'src/libsyntax') diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index cf8f8abe2ab..e7d79a647d3 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 1abc7832ffa..94bbd5ba2f7 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 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); } } diff --git a/src/libsyntax/util/parser.rs b/src/libsyntax/util/parser.rs index 5f15ede7b0b..d76dede8155 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; -- 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') 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') 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') 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 6d26c5f73ca1c6d1615c141b181374e152cab61a Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Mon, 6 May 2019 13:42:15 -0500 Subject: rustfmt --- src/libsyntax/ext/tt/transcribe.rs | 100 ++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 50 deletions(-) (limited to 'src/libsyntax') diff --git a/src/libsyntax/ext/tt/transcribe.rs b/src/libsyntax/ext/tt/transcribe.rs index bd2adb5ac13..5eb5475b2a9 100644 --- a/src/libsyntax/ext/tt/transcribe.rs +++ b/src/libsyntax/ext/tt/transcribe.rs @@ -1,10 +1,10 @@ use crate::ast::Ident; use crate::ext::base::ExtCtxt; use crate::ext::expand::Marker; -use crate::ext::tt::macro_parser::{NamedMatch, MatchedSeq, MatchedNonterminal}; +use crate::ext::tt::macro_parser::{MatchedNonterminal, MatchedSeq, NamedMatch}; use crate::ext::tt::quoted; use crate::mut_visit::noop_visit_tt; -use crate::parse::token::{self, Token, NtTT}; +use crate::parse::token::{self, NtTT, Token}; use crate::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint}; use smallvec::{smallvec, SmallVec}; @@ -16,18 +16,10 @@ use std::mem; use std::ops::Add; use std::rc::Rc; -// An iterator over the token trees in a delimited token tree (`{ ... }`) or a sequence (`$(...)`). +/// An iterator over the token trees in a delimited token tree (`{ ... }`) or a sequence (`$(...)`). enum Frame { - Delimited { - forest: Lrc, - idx: usize, - span: DelimSpan, - }, - Sequence { - forest: Lrc, - idx: usize, - sep: Option, - }, + Delimited { forest: Lrc, idx: usize, span: DelimSpan }, + Sequence { forest: Lrc, idx: usize, sep: Option }, } impl Frame { @@ -54,13 +46,13 @@ impl Iterator for Frame { } } -/// This can do Macro-By-Example transcription. On the other hand, if -/// `src` contains no `TokenTree::{Sequence, MetaVar, MetaVarDecl}`s, `interp` can -/// (and should) be None. -pub fn transcribe(cx: &ExtCtxt<'_>, - interp: Option>>, - src: Vec) - -> TokenStream { +/// This can do Macro-By-Example transcription. On the other hand, if `src` contains no +/// `TokenTree::{Sequence, MetaVar, MetaVarDecl}`s, `interp` can (and should) be `None`. +pub fn transcribe( + cx: &ExtCtxt<'_>, + interp: Option>>, + src: Vec, +) -> TokenStream { let mut stack: SmallVec<[Frame; 1]> = smallvec![Frame::new(src)]; let interpolations = interp.unwrap_or_else(FxHashMap::default); /* just a convenience */ let mut repeats = Vec::new(); @@ -84,7 +76,7 @@ pub fn transcribe(cx: &ExtCtxt<'_>, }; result.push(TokenTree::Token(prev_span, sep).into()); } - continue + continue; } } @@ -96,29 +88,30 @@ pub fn transcribe(cx: &ExtCtxt<'_>, if result_stack.is_empty() { return TokenStream::new(result); } - let tree = TokenTree::Delimited( - span, - forest.delim, - TokenStream::new(result).into(), - ); + let tree = + TokenTree::Delimited(span, forest.delim, TokenStream::new(result).into()); result = result_stack.pop().unwrap(); result.push(tree.into()); } } - continue + continue; }; match tree { quoted::TokenTree::Sequence(sp, seq) => { // FIXME(pcwalton): Bad copy. - match lockstep_iter_size("ed::TokenTree::Sequence(sp, seq.clone()), - &interpolations, - &repeats) { + match lockstep_iter_size( + "ed::TokenTree::Sequence(sp, seq.clone()), + &interpolations, + &repeats, + ) { LockstepIterSize::Unconstrained => { - cx.span_fatal(sp.entire(), /* blame macro writer */ + cx.span_fatal( + sp.entire(), /* blame macro writer */ "attempted to repeat an expression \ containing no syntax \ - variables matched as repeating at this depth"); + variables matched as repeating at this depth", + ); } LockstepIterSize::Contradiction(ref msg) => { // FIXME #2887 blame macro invoker instead @@ -153,8 +146,10 @@ pub fn transcribe(cx: &ExtCtxt<'_>, result.push(token.into()); } } else { - cx.span_fatal(sp, /* blame the macro writer */ - &format!("variable '{}' is still repeating at this depth", ident)); + cx.span_fatal( + sp, /* blame the macro writer */ + &format!("variable '{}' is still repeating at this depth", ident), + ); } } else { let ident = @@ -180,10 +175,11 @@ pub fn transcribe(cx: &ExtCtxt<'_>, } } -fn lookup_cur_matched(ident: Ident, - interpolations: &FxHashMap>, - repeats: &[(usize, usize)]) - -> Option> { +fn lookup_cur_matched( + ident: Ident, + interpolations: &FxHashMap>, + repeats: &[(usize, usize)], +) -> Option> { interpolations.get(&ident).map(|matched| { let mut matched = matched.clone(); for &(idx, _) in repeats { @@ -217,9 +213,11 @@ impl Add for LockstepIterSize { LockstepIterSize::Contradiction(_) => other, LockstepIterSize::Constraint(r_len, _) if l_len == r_len => self, LockstepIterSize::Constraint(r_len, r_id) => { - let msg = format!("inconsistent lockstep iteration: \ - '{}' has {} items, but '{}' has {}", - l_id, l_len, r_id, r_len); + let msg = format!( + "inconsistent lockstep iteration: \ + '{}' has {} items, but '{}' has {}", + l_id, l_len, r_id, r_len + ); LockstepIterSize::Contradiction(msg) } }, @@ -227,30 +225,32 @@ impl Add for LockstepIterSize { } } -fn lockstep_iter_size(tree: "ed::TokenTree, - interpolations: &FxHashMap>, - repeats: &[(usize, usize)]) - -> LockstepIterSize { +fn lockstep_iter_size( + tree: "ed::TokenTree, + interpolations: &FxHashMap>, + repeats: &[(usize, usize)], +) -> LockstepIterSize { use quoted::TokenTree; match *tree { TokenTree::Delimited(_, ref delimed) => { delimed.tts.iter().fold(LockstepIterSize::Unconstrained, |size, tt| { size + lockstep_iter_size(tt, interpolations, repeats) }) - }, + } TokenTree::Sequence(_, ref seq) => { seq.tts.iter().fold(LockstepIterSize::Unconstrained, |size, tt| { size + lockstep_iter_size(tt, interpolations, repeats) }) - }, - TokenTree::MetaVar(_, name) | TokenTree::MetaVarDecl(_, name, _) => + } + TokenTree::MetaVar(_, name) | TokenTree::MetaVarDecl(_, name, _) => { match lookup_cur_matched(name, interpolations, repeats) { Some(matched) => match *matched { MatchedNonterminal(_) => LockstepIterSize::Unconstrained, MatchedSeq(ref ads, _) => LockstepIterSize::Constraint(ads.len(), name), }, - _ => LockstepIterSize::Unconstrained - }, + _ => LockstepIterSize::Unconstrained, + } + } TokenTree::Token(..) => LockstepIterSize::Unconstrained, } } -- 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') 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 606bb6f6fe4cfb6e3054e90cc400553f280ad94a Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Mon, 6 May 2019 16:31:59 -0500 Subject: avoid extra copy --- src/libsyntax/ext/tt/transcribe.rs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) (limited to 'src/libsyntax') diff --git a/src/libsyntax/ext/tt/transcribe.rs b/src/libsyntax/ext/tt/transcribe.rs index 5eb5475b2a9..424b94173a7 100644 --- a/src/libsyntax/ext/tt/transcribe.rs +++ b/src/libsyntax/ext/tt/transcribe.rs @@ -98,16 +98,11 @@ pub fn transcribe( }; match tree { - quoted::TokenTree::Sequence(sp, seq) => { - // FIXME(pcwalton): Bad copy. - match lockstep_iter_size( - "ed::TokenTree::Sequence(sp, seq.clone()), - &interpolations, - &repeats, - ) { + seq @ quoted::TokenTree::Sequence(..) => { + match lockstep_iter_size(&seq, interp, &repeats) { LockstepIterSize::Unconstrained => { cx.span_fatal( - sp.entire(), /* blame macro writer */ + seq.span(), /* blame macro writer */ "attempted to repeat an expression \ containing no syntax \ variables matched as repeating at this depth", @@ -115,9 +110,15 @@ pub fn transcribe( } LockstepIterSize::Contradiction(ref msg) => { // FIXME #2887 blame macro invoker instead - cx.span_fatal(sp.entire(), &msg[..]); + cx.span_fatal(seq.span(), &msg[..]); } LockstepIterSize::Constraint(len, _) => { + let (sp, seq) = if let quoted::TokenTree::Sequence(sp, seq) = seq { + (sp, seq) + } else { + unreachable!() + }; + if len == 0 { if seq.op == quoted::KleeneOp::OneOrMore { // FIXME #2887 blame invoker @@ -201,10 +202,8 @@ enum LockstepIterSize { Contradiction(String), } -impl Add for LockstepIterSize { - type Output = LockstepIterSize; - - fn add(self, other: LockstepIterSize) -> LockstepIterSize { +impl LockstepIterSize { + fn with(self, other: LockstepIterSize) -> LockstepIterSize { match self { LockstepIterSize::Unconstrained => other, LockstepIterSize::Contradiction(_) => self, -- cgit 1.4.1-3-g733a5 From 5db665b972f7c9578fe0fd2bdb9c7aef6f6db3db Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Mon, 6 May 2019 16:16:23 -0500 Subject: lots of comments + minor cleanup --- src/libsyntax/ext/tt/macro_parser.rs | 5 +- src/libsyntax/ext/tt/macro_rules.rs | 2 +- src/libsyntax/ext/tt/quoted.rs | 1 + src/libsyntax/ext/tt/transcribe.rs | 147 +++++++++++++++++++++++++++++++---- 4 files changed, 139 insertions(+), 16 deletions(-) (limited to 'src/libsyntax') diff --git a/src/libsyntax/ext/tt/macro_parser.rs b/src/libsyntax/ext/tt/macro_parser.rs index ab5823eaca5..084a69f4cda 100644 --- a/src/libsyntax/ext/tt/macro_parser.rs +++ b/src/libsyntax/ext/tt/macro_parser.rs @@ -554,7 +554,10 @@ fn inner_parse_loop<'root, 'tt>( match item.top_elts.get_tt(idx) { // Need to descend into a sequence TokenTree::Sequence(sp, seq) => { - // Examine the case where there are 0 matches of this sequence + // Examine the case where there are 0 matches of this sequence. We are + // implicitly disallowing OneOrMore from having 0 matches here. Thus, that will + // result in a "no rules expected token" error by virtue of this matcher not + // working. if seq.op == quoted::KleeneOp::ZeroOrMore || seq.op == quoted::KleeneOp::ZeroOrOne { diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index b1b9d25b3d5..a53cc2fe661 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -151,7 +151,7 @@ fn generic_extension<'cx>(cx: &'cx mut ExtCtxt<'_>, let rhs_spans = rhs.iter().map(|t| t.span()).collect::>(); // rhs has holes ( `$id` and `$(...)` that need filled) - let mut tts = transcribe(cx, Some(named_matches), rhs); + let mut tts = transcribe(cx, &named_matches, rhs); // Replace all the tokens for the corresponding positions in the macro, to maintain // proper positions in error reporting, while maintaining the macro_backtrace. diff --git a/src/libsyntax/ext/tt/quoted.rs b/src/libsyntax/ext/tt/quoted.rs index b24edb57e52..ed8395f11ad 100644 --- a/src/libsyntax/ext/tt/quoted.rs +++ b/src/libsyntax/ext/tt/quoted.rs @@ -73,6 +73,7 @@ pub enum KleeneOp { ZeroOrMore, /// Kleene plus (`+`) for one or more repetitions OneOrMore, + /// Kleene optional (`?`) for zero or one reptitions ZeroOrOne, } diff --git a/src/libsyntax/ext/tt/transcribe.rs b/src/libsyntax/ext/tt/transcribe.rs index 424b94173a7..3a40e8403cc 100644 --- a/src/libsyntax/ext/tt/transcribe.rs +++ b/src/libsyntax/ext/tt/transcribe.rs @@ -13,7 +13,6 @@ use syntax_pos::DUMMY_SP; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::Lrc; use std::mem; -use std::ops::Add; use std::rc::Rc; /// An iterator over the token trees in a delimited token tree (`{ ... }`) or a sequence (`$(...)`). @@ -23,6 +22,7 @@ enum Frame { } impl Frame { + /// Construct a new frame around the delimited set of tokens. fn new(tts: Vec) -> Frame { let forest = Lrc::new(quoted::Delimited { delim: token::NoDelim, tts: tts }); Frame::Delimited { forest: forest, idx: 0, span: DelimSpan::dummy() } @@ -46,30 +46,72 @@ impl Iterator for Frame { } } -/// This can do Macro-By-Example transcription. On the other hand, if `src` contains no -/// `TokenTree::{Sequence, MetaVar, MetaVarDecl}`s, `interp` can (and should) be `None`. +/// This can do Macro-By-Example transcription. +/// - `interp` is a map of meta-variables to the tokens (non-terminals) they matched in the +/// invocation. We are assuming we already know there is a match. +/// - `src` is the RHS of the MBE, that is, the "example" we are filling in. +/// +/// For example, +/// +/// ```rust +/// macro_rules! foo { +/// ($id:ident) => { println!("{}", stringify!($id)); } +/// } +/// +/// foo!(bar); +/// ``` +/// +/// `interp` would contain `$id => bar` and `src` would contain `println!("{}", stringify!($id));`. +/// +/// `transcribe` would return a `TokenStream` containing `println!("{}", stringify!(bar));`. +/// +/// Along the way, we do some additional error checking. pub fn transcribe( cx: &ExtCtxt<'_>, - interp: Option>>, + interp: &FxHashMap>, src: Vec, ) -> TokenStream { + assert!(src.len() > 0); + + // We descend into the RHS (`src`), expanding things as we go. This stack contains the things + // we have yet to expand/are still expanding. We start the stack off with the whole RHS. let mut stack: SmallVec<[Frame; 1]> = smallvec![Frame::new(src)]; - let interpolations = interp.unwrap_or_else(FxHashMap::default); /* just a convenience */ + + // As we descend in the RHS, we will need to be able to match nested sequences of matchers. + // `repeats` keeps track of where we are in matching at each level, with the last element being + // the most deeply nested sequence. This is used as a stack. let mut repeats = Vec::new(); + + // `result` contains resulting token stream from the TokenTree we just finished processing. At + // the end, this will contain the full result of transcription, but at arbitrary points during + // `transcribe`, `result` will contain subsets of the final result. + // + // Specifically, as we descend into each TokenTree, we will push the existing results onto the + // `result_stack` and clear `results`. We will then produce the results of transcribing the + // TokenTree into `results`. Then, as we unwind back out of the `TokenTree`, we will pop the + // `result_stack` and append `results` too it to produce the new `results` up to that point. + // + // Thus, if we try to pop the `result_stack` and it is empty, we have reached the top-level + // again, and we are done transcribing. let mut result: Vec = Vec::new(); let mut result_stack = Vec::new(); loop { + // Look at the last frame on the stack. let tree = if let Some(tree) = stack.last_mut().unwrap().next() { + // If it still has a TokenTree we have not looked at yet, use that tree. tree - } else { + } + // The else-case never produces a value for `tree` (it `continue`s or `return`s). + else { + // Otherwise, if we have just reached the end of a sequence and we can keep repeating, + // go back to the beginning of the sequence. if let Frame::Sequence { ref mut idx, ref sep, .. } = *stack.last_mut().unwrap() { let (ref mut repeat_idx, repeat_len) = *repeats.last_mut().unwrap(); *repeat_idx += 1; if *repeat_idx < repeat_len { *idx = 0; if let Some(sep) = sep.clone() { - // repeat same span, I guess let prev_span = match result.last() { Some((tt, _)) => tt.span(), None => DUMMY_SP, @@ -80,14 +122,25 @@ pub fn transcribe( } } + // We are done with the top of the stack. Pop it. Depending on what it was, we do + // different things. Note that the outermost item must be the delimited, wrapped RHS + // that was passed in originally to `transcribe`. match stack.pop().unwrap() { + // Done with a sequence. Pop from repeats. Frame::Sequence { .. } => { repeats.pop(); } + + // We are done processing a Delimited. If this is the top-level delimited, we are + // done. Otherwise, we unwind the result_stack to append what we have produced to + // any previous results. Frame::Delimited { forest, span, .. } => { if result_stack.is_empty() { + // No results left to compute! We are back at the top-level. return TokenStream::new(result); } + + // Step back into the parent Delimited. let tree = TokenTree::Delimited(span, forest.delim, TokenStream::new(result).into()); result = result_stack.pop().unwrap(); @@ -97,35 +150,54 @@ pub fn transcribe( continue; }; + // At this point, we know we are in the middle of a TokenTree (the last one on `stack`). + // `tree` contains the next `TokenTree` to be processed. match tree { + // We are descending into a sequence. We first make sure that the matchers in the RHS + // and the matches in `interp` have the same shape. Otherwise, either the caller or the + // macro writer has made a mistake. seq @ quoted::TokenTree::Sequence(..) => { match lockstep_iter_size(&seq, interp, &repeats) { LockstepIterSize::Unconstrained => { cx.span_fatal( seq.span(), /* blame macro writer */ - "attempted to repeat an expression \ - containing no syntax \ - variables matched as repeating at this depth", + "attempted to repeat an expression containing no syntax variables \ + matched as repeating at this depth", ); } + LockstepIterSize::Contradiction(ref msg) => { + // FIXME: this should be impossible. I (mark-i-m) believe it would + // represent a bug in the macro_parser. // FIXME #2887 blame macro invoker instead cx.span_fatal(seq.span(), &msg[..]); } + LockstepIterSize::Constraint(len, _) => { + // We do this to avoid an extra clone above. We know that this is a + // sequence already. let (sp, seq) = if let quoted::TokenTree::Sequence(sp, seq) = seq { (sp, seq) } else { unreachable!() }; + // Is the repetition empty? if len == 0 { if seq.op == quoted::KleeneOp::OneOrMore { + // FIXME: this should be impossible because we check for this in + // macro_parser.rs // FIXME #2887 blame invoker cx.span_fatal(sp.entire(), "this must repeat at least once"); } } else { + // 0 is the initial counter (we have done 0 repretitions so far). `len` + // is the total number of reptitions we should generate. repeats.push((0, len)); + + // The first time we encounter the sequence we push it to the stack. It + // then gets reused (see the beginning of the loop) until we are done + // repeating. stack.push(Frame::Sequence { idx: 0, sep: seq.separator.clone(), @@ -135,10 +207,16 @@ pub fn transcribe( } } } - // FIXME #2887: think about span stuff here + + // Replace the meta-var with the matched token tree from the invocation. quoted::TokenTree::MetaVar(mut sp, ident) => { - if let Some(cur_matched) = lookup_cur_matched(ident, &interpolations, &repeats) { + // Find the matched nonterminal from the macro invocation, and use it to replace + // the meta-var. + if let Some(cur_matched) = lookup_cur_matched(ident, interp, &repeats) { if let MatchedNonterminal(ref nt) = *cur_matched { + // FIXME #2887: why do we apply a mark when matching a token tree meta-var + // (e.g. `$x:tt`), but not when we are matching any other type of token + // tree? if let NtTT(ref tt) = **nt { result.push(tt.clone().into()); } else { @@ -147,12 +225,15 @@ pub fn transcribe( result.push(token.into()); } } else { + // We were unable to descend far enough. This is an error. cx.span_fatal( sp, /* blame the macro writer */ &format!("variable '{}' is still repeating at this depth", ident), ); } } else { + // If we aren't able to match the meta-var, we push it back into the result but + // with modified syntax context. (I believe this supports nested macros). let ident = Ident::new(ident.name, ident.span.apply_mark(cx.current_expansion.mark)); sp = sp.apply_mark(cx.current_expansion.mark); @@ -160,22 +241,39 @@ pub fn transcribe( result.push(TokenTree::Token(sp, token::Token::from_ast_ident(ident)).into()); } } + + // If we are entering a new delimiter, we push its contents to the `stack` to be + // processed, and we push all of the currently produced results to the `result_stack`. + // We will produce all of the results of the inside of the `Delimited` and then we will + // jump back out of the Delimited, pop the result_stack and add the new results back to + // the previous results (from outside the Delimited). quoted::TokenTree::Delimited(mut span, delimited) => { span = span.apply_mark(cx.current_expansion.mark); stack.push(Frame::Delimited { forest: delimited, idx: 0, span: span }); result_stack.push(mem::replace(&mut result, Vec::new())); } + + // Nothing much to do here. Just push the token to the result, being careful to + // preserve syntax context. quoted::TokenTree::Token(sp, tok) => { let mut marker = Marker(cx.current_expansion.mark); let mut tt = TokenTree::Token(sp, tok); noop_visit_tt(&mut tt, &mut marker); result.push(tt.into()); } + + // There should be no meta-var declarations in the invocation of a macro. quoted::TokenTree::MetaVarDecl(..) => panic!("unexpected `TokenTree::MetaVarDecl"), } } } +/// Lookup the meta-var named `ident` and return the matched token tree from the invocation using +/// the set of matches `interpolations`. +/// +/// See the definition of `repeats` in the `transcribe` function. `repeats` is used to descend +/// into the right place in nested matchers. If we attempt to descend too far, the macro writer has +/// made a mistake, and we return `None`. fn lookup_cur_matched( ident: Ident, interpolations: &FxHashMap>, @@ -195,14 +293,29 @@ fn lookup_cur_matched( }) } +/// An accumulator over a TokenTree to be used with `fold`. During transcription, we need to make +/// sure that the size of each sequence and all of its nested sequences are the same as the sizes +/// of all the matched (nested) sequences in the macro invocation. If they don't match, somebody +/// has made a mistake (either the macro writer or caller). #[derive(Clone)] enum LockstepIterSize { + /// No constraints on length of matcher. This is true for any TokenTree variants except a + /// `MetaVar` with an actual `MatchedSeq` (as opposed to a `MatchedNonterminal`). Unconstrained, + + /// A `MetaVar` with an actual `MatchedSeq`. The length of the match and the name of the + /// meta-var are returned. Constraint(usize, Ident), + + /// Two `Constraint`s on the same sequence had different lengths. This is an error. Contradiction(String), } impl LockstepIterSize { + /// Find incompatibilities in matcher/invocation sizes. + /// - `Unconstrained` is compatible with everything. + /// - `Contradiction` is incompatible with everything. + /// - `Constraint(len)` is only compatible with other constraints of the same length. fn with(self, other: LockstepIterSize) -> LockstepIterSize { match self { LockstepIterSize::Unconstrained => other, @@ -224,6 +337,12 @@ impl LockstepIterSize { } } +/// Given a `tree`, make sure that all sequences have the same length as the matches for the +/// appropriate meta-vars in `interpolations`. +/// +/// Note that if `repeats` does not match the exact correct depth of a meta-var, +/// `lookup_cur_matched` will return `None`, which is why this still works even in the presnece of +/// multiple nested matcher sequences. fn lockstep_iter_size( tree: "ed::TokenTree, interpolations: &FxHashMap>, @@ -233,12 +352,12 @@ fn lockstep_iter_size( match *tree { TokenTree::Delimited(_, ref delimed) => { delimed.tts.iter().fold(LockstepIterSize::Unconstrained, |size, tt| { - size + lockstep_iter_size(tt, interpolations, repeats) + size.with(lockstep_iter_size(tt, interpolations, repeats)) }) } TokenTree::Sequence(_, ref seq) => { seq.tts.iter().fold(LockstepIterSize::Unconstrained, |size, tt| { - size + lockstep_iter_size(tt, interpolations, repeats) + size.with(lockstep_iter_size(tt, interpolations, repeats)) }) } TokenTree::MetaVar(_, name) | TokenTree::MetaVarDecl(_, name, _) => { -- cgit 1.4.1-3-g733a5 From eb7d47cba90ac716c84fe720418e58c6eb84ac4e Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 8 May 2019 13:34:23 -0500 Subject: fix incorrect assert --- src/libsyntax/ext/tt/transcribe.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/libsyntax') diff --git a/src/libsyntax/ext/tt/transcribe.rs b/src/libsyntax/ext/tt/transcribe.rs index 3a40e8403cc..0cefcf1ce03 100644 --- a/src/libsyntax/ext/tt/transcribe.rs +++ b/src/libsyntax/ext/tt/transcribe.rs @@ -71,7 +71,10 @@ pub fn transcribe( interp: &FxHashMap>, src: Vec, ) -> TokenStream { - assert!(src.len() > 0); + // Nothing for us to transcribe... + if src.is_empty() { + return TokenStream::empty(); + } // We descend into the RHS (`src`), expanding things as we go. This stack contains the things // we have yet to expand/are still expanding. We start the stack off with the whole RHS. -- 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') 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') 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