From 29fb07d2451035cce7fbe99bc854cb3d71b5f1a1 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 14 Oct 2019 16:47:12 +0200 Subject: [PATCH 1/5] syntax: add recovery for intersection patterns `p1 @ p2` --- src/libsyntax/parse/parser/pat.rs | 60 +++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 48f9e30161031..c0e577bd58b1d 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -367,6 +367,7 @@ impl<'a> Parser<'a> { let pat = self.mk_pat(lo.to(self.prev_span), pat); let pat = self.maybe_recover_from_bad_qpath(pat, true)?; + let pat = self.recover_intersection_pat(pat)?; if !allow_range_pat { self.ban_pat_range_if_ambiguous(&pat)? @@ -375,6 +376,65 @@ impl<'a> Parser<'a> { Ok(pat) } + /// Try to recover the more general form `intersect ::= $pat_lhs @ $pat_rhs`. + /// + /// Allowed binding patterns generated by `binding ::= ref? mut? $ident @ $pat_rhs` + /// should already have been parsed by now at this point, + /// if the next token is `@` then we can try to parse the more general form. + /// + /// Consult `parse_pat_ident` for the `binding` grammar. + /// + /// The notion of intersection patterns are found in + /// e.g. [F#][and] where they are called AND-patterns. + /// + /// [and]: https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/pattern-matching + fn recover_intersection_pat(&mut self, lhs: P) -> PResult<'a, P> { + if self.token.kind != token::At { + // Next token is not `@` so it's not going to be an intersection pattern. + return Ok(lhs); + } + + // At this point we attempt to parse `@ $pat_rhs` and emit an error. + self.bump(); // `@` + let mut rhs = self.parse_pat(None)?; + let sp = lhs.span.to(rhs.span); + + if let PatKind::Ident(_, _, ref mut sub @ None) = rhs.kind { + // The user inverted the order, so help them fix that. + let mut applicability = Applicability::MachineApplicable; + lhs.walk(&mut |p| match p.kind { + // `check_match` is unhappy if the subpattern has a binding anywhere. + PatKind::Ident(..) => { + applicability = Applicability::MaybeIncorrect; + false // Short-circuit. + }, + _ => true, + }); + + let lhs_span = lhs.span; + // Move the LHS into the RHS as a subpattern. + // The RHS is now the full pattern. + *sub = Some(lhs); + + self.struct_span_err(sp, "pattern on wrong side of `@`") + .span_label(lhs_span, "pattern on the left, should be to the right") + .span_label(rhs.span, "binding on the right, should be to the left") + .span_suggestion(sp, "switch the order", pprust::pat_to_string(&rhs), applicability) + .emit(); + + rhs.span = sp; + return Ok(rhs); + } + + // The special case above doesn't apply so we may have e.g. `A(x) @ B(y)`. + let mut err = self.struct_span_err(sp, "left-hand side of `@` must be a binding pattern"); + err.span_label(lhs.span, "interpreted as a pattern, not a binding") + .span_label(rhs.span, "also a pattern") + .note("bindings are `x`, `mut x`, `ref x`, and `ref mut x`"); + // FIXME(Centril): Introduce `PatKind::Err` and use that instead. + Err(err) + } + /// Ban a range pattern if it has an ambiguous interpretation. fn ban_pat_range_if_ambiguous(&self, pat: &Pat) -> PResult<'a, ()> { match pat.kind { From a77a8aaa2e55c0591ee766f3ff2142a67439d243 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 14 Oct 2019 16:57:18 +0200 Subject: [PATCH 2/5] syntax: add test for intersection pattern parser recovery --- src/test/ui/parser/intersection-patterns.rs | 40 +++++++++++++++++++ .../ui/parser/intersection-patterns.stderr | 33 +++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 src/test/ui/parser/intersection-patterns.rs create mode 100644 src/test/ui/parser/intersection-patterns.stderr diff --git a/src/test/ui/parser/intersection-patterns.rs b/src/test/ui/parser/intersection-patterns.rs new file mode 100644 index 0000000000000..1dda21519e393 --- /dev/null +++ b/src/test/ui/parser/intersection-patterns.rs @@ -0,0 +1,40 @@ +// This tests the parser recovery in `recover_intersection_pat` +// and serves as a regression test for the diagnostics issue #65400. +// +// The general idea is that for `$pat_lhs @ $pat_rhs` where +// `$pat_lhs` is not generated by `ref? mut? $ident` we want +// to suggest either switching the order or note that intersection +// patterns are not allowed. + +fn main() { + let s: Option = None; + + match s { + Some(x) @ y => {} + //~^ ERROR pattern on wrong side of `@` + //~| pattern on the left, should be to the right + //~| binding on the right, should be to the left + //~| HELP switch the order + //~| SUGGESTION y@Some(x) + _ => {} + } + + match s { + Some(x) @ Some(y) => {} + //~^ ERROR left-hand side of `@` must be a binding pattern + //~| interpreted as a pattern, not a binding + //~| also a pattern + //~| NOTE bindings are `x`, `mut x`, `ref x`, and `ref mut x` + _ => {} + } + + match 2 { + 1 ..= 5 @ e => {} + //~^ ERROR pattern on wrong side of `@` + //~| pattern on the left, should be to the right + //~| binding on the right, should be to the left + //~| HELP switch the order + //~| SUGGESTION e@1 ..=5 + _ => {} + } +} diff --git a/src/test/ui/parser/intersection-patterns.stderr b/src/test/ui/parser/intersection-patterns.stderr new file mode 100644 index 0000000000000..03781f8fdee30 --- /dev/null +++ b/src/test/ui/parser/intersection-patterns.stderr @@ -0,0 +1,33 @@ +error: pattern on wrong side of `@` + --> $DIR/intersection-patterns.rs:13:9 + | +LL | Some(x) @ y => {} + | -------^^^- + | | | + | | binding on the right, should be to the left + | pattern on the left, should be to the right + | help: switch the order: `y@Some(x)` + +error: left-hand side of `@` must be a binding pattern + --> $DIR/intersection-patterns.rs:23:9 + | +LL | Some(x) @ Some(y) => {} + | -------^^^------- + | | | + | | also a pattern + | interpreted as a pattern, not a binding + | + = note: bindings are `x`, `mut x`, `ref x`, and `ref mut x` + +error: pattern on wrong side of `@` + --> $DIR/intersection-patterns.rs:32:9 + | +LL | 1 ..= 5 @ e => {} + | -------^^^- + | | | + | | binding on the right, should be to the left + | pattern on the left, should be to the right + | help: switch the order: `e@1 ..=5` + +error: aborting due to 3 previous errors + From 72ad8f716bc2591fba72d7107a9938952214f485 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 14 Oct 2019 17:25:50 +0200 Subject: [PATCH 3/5] syntax: use `PatKind::Wild` as our `::Err` equivalent. --- src/libsyntax/parse/parser/pat.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index c0e577bd58b1d..55c534e56ceb6 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -421,18 +421,18 @@ impl<'a> Parser<'a> { .span_label(rhs.span, "binding on the right, should be to the left") .span_suggestion(sp, "switch the order", pprust::pat_to_string(&rhs), applicability) .emit(); - - rhs.span = sp; - return Ok(rhs); + } else { + // The special case above doesn't apply so we may have e.g. `A(x) @ B(y)`. + rhs.kind = PatKind::Wild; + self.struct_span_err(sp, "left-hand side of `@` must be a binding pattern") + .span_label(lhs.span, "interpreted as a pattern, not a binding") + .span_label(rhs.span, "also a pattern") + .note("bindings are `x`, `mut x`, `ref x`, and `ref mut x`") + .emit(); } - // The special case above doesn't apply so we may have e.g. `A(x) @ B(y)`. - let mut err = self.struct_span_err(sp, "left-hand side of `@` must be a binding pattern"); - err.span_label(lhs.span, "interpreted as a pattern, not a binding") - .span_label(rhs.span, "also a pattern") - .note("bindings are `x`, `mut x`, `ref x`, and `ref mut x`"); - // FIXME(Centril): Introduce `PatKind::Err` and use that instead. - Err(err) + rhs.span = sp; + Ok(rhs) } /// Ban a range pattern if it has an ambiguous interpretation. From 3a9f8deb1d568baeb2cac503cf8667e3170fb2f8 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 14 Oct 2019 18:02:49 +0200 Subject: [PATCH 4/5] recover_intersection_pat: adjust wording --- src/libsyntax/parse/parser/pat.rs | 6 +++--- src/test/ui/parser/intersection-patterns.rs | 10 +++++----- src/test/ui/parser/intersection-patterns.stderr | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 55c534e56ceb6..e288346a32927 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -417,14 +417,14 @@ impl<'a> Parser<'a> { *sub = Some(lhs); self.struct_span_err(sp, "pattern on wrong side of `@`") - .span_label(lhs_span, "pattern on the left, should be to the right") - .span_label(rhs.span, "binding on the right, should be to the left") + .span_label(lhs_span, "pattern on the left, should be on the right") + .span_label(rhs.span, "binding on the right, should be on the left") .span_suggestion(sp, "switch the order", pprust::pat_to_string(&rhs), applicability) .emit(); } else { // The special case above doesn't apply so we may have e.g. `A(x) @ B(y)`. rhs.kind = PatKind::Wild; - self.struct_span_err(sp, "left-hand side of `@` must be a binding pattern") + self.struct_span_err(sp, "left-hand side of `@` must be a binding") .span_label(lhs.span, "interpreted as a pattern, not a binding") .span_label(rhs.span, "also a pattern") .note("bindings are `x`, `mut x`, `ref x`, and `ref mut x`") diff --git a/src/test/ui/parser/intersection-patterns.rs b/src/test/ui/parser/intersection-patterns.rs index 1dda21519e393..3b0dfd1ee99be 100644 --- a/src/test/ui/parser/intersection-patterns.rs +++ b/src/test/ui/parser/intersection-patterns.rs @@ -12,8 +12,8 @@ fn main() { match s { Some(x) @ y => {} //~^ ERROR pattern on wrong side of `@` - //~| pattern on the left, should be to the right - //~| binding on the right, should be to the left + //~| pattern on the left, should be on the right + //~| binding on the right, should be on the left //~| HELP switch the order //~| SUGGESTION y@Some(x) _ => {} @@ -21,7 +21,7 @@ fn main() { match s { Some(x) @ Some(y) => {} - //~^ ERROR left-hand side of `@` must be a binding pattern + //~^ ERROR left-hand side of `@` must be a binding //~| interpreted as a pattern, not a binding //~| also a pattern //~| NOTE bindings are `x`, `mut x`, `ref x`, and `ref mut x` @@ -31,8 +31,8 @@ fn main() { match 2 { 1 ..= 5 @ e => {} //~^ ERROR pattern on wrong side of `@` - //~| pattern on the left, should be to the right - //~| binding on the right, should be to the left + //~| pattern on the left, should be on the right + //~| binding on the right, should be on the left //~| HELP switch the order //~| SUGGESTION e@1 ..=5 _ => {} diff --git a/src/test/ui/parser/intersection-patterns.stderr b/src/test/ui/parser/intersection-patterns.stderr index 03781f8fdee30..39fc9b9475d91 100644 --- a/src/test/ui/parser/intersection-patterns.stderr +++ b/src/test/ui/parser/intersection-patterns.stderr @@ -4,11 +4,11 @@ error: pattern on wrong side of `@` LL | Some(x) @ y => {} | -------^^^- | | | - | | binding on the right, should be to the left - | pattern on the left, should be to the right + | | binding on the right, should be on the left + | pattern on the left, should be on the right | help: switch the order: `y@Some(x)` -error: left-hand side of `@` must be a binding pattern +error: left-hand side of `@` must be a binding --> $DIR/intersection-patterns.rs:23:9 | LL | Some(x) @ Some(y) => {} @@ -25,8 +25,8 @@ error: pattern on wrong side of `@` LL | 1 ..= 5 @ e => {} | -------^^^- | | | - | | binding on the right, should be to the left - | pattern on the left, should be to the right + | | binding on the right, should be on the left + | pattern on the left, should be on the right | help: switch the order: `e@1 ..=5` error: aborting due to 3 previous errors From 16266a54058a71c943d064054bfe3a1b5704a444 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 14 Oct 2019 18:12:04 +0200 Subject: [PATCH 5/5] pprust: `p1@p2` -> `p1 @ p2` --- src/libsyntax/print/pprust.rs | 3 ++- src/test/ui/parser/intersection-patterns.rs | 4 ++-- src/test/ui/parser/intersection-patterns.stderr | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 7d4ffe493d709..68dd90b54ab68 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -2381,7 +2381,8 @@ impl<'a> State<'a> { } self.print_ident(ident); if let Some(ref p) = *sub { - self.s.word("@"); + self.s.space(); + self.s.word_space("@"); self.print_pat(p); } } diff --git a/src/test/ui/parser/intersection-patterns.rs b/src/test/ui/parser/intersection-patterns.rs index 3b0dfd1ee99be..adb607cf6b97b 100644 --- a/src/test/ui/parser/intersection-patterns.rs +++ b/src/test/ui/parser/intersection-patterns.rs @@ -15,7 +15,7 @@ fn main() { //~| pattern on the left, should be on the right //~| binding on the right, should be on the left //~| HELP switch the order - //~| SUGGESTION y@Some(x) + //~| SUGGESTION y @ Some(x) _ => {} } @@ -34,7 +34,7 @@ fn main() { //~| pattern on the left, should be on the right //~| binding on the right, should be on the left //~| HELP switch the order - //~| SUGGESTION e@1 ..=5 + //~| SUGGESTION e @ 1 ..=5 _ => {} } } diff --git a/src/test/ui/parser/intersection-patterns.stderr b/src/test/ui/parser/intersection-patterns.stderr index 39fc9b9475d91..f5bfee5bbd611 100644 --- a/src/test/ui/parser/intersection-patterns.stderr +++ b/src/test/ui/parser/intersection-patterns.stderr @@ -6,7 +6,7 @@ LL | Some(x) @ y => {} | | | | | binding on the right, should be on the left | pattern on the left, should be on the right - | help: switch the order: `y@Some(x)` + | help: switch the order: `y @ Some(x)` error: left-hand side of `@` must be a binding --> $DIR/intersection-patterns.rs:23:9 @@ -27,7 +27,7 @@ LL | 1 ..= 5 @ e => {} | | | | | binding on the right, should be on the left | pattern on the left, should be on the right - | help: switch the order: `e@1 ..=5` + | help: switch the order: `e @ 1 ..=5` error: aborting due to 3 previous errors