Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syntax: add parser recovery for intersection- / and-patterns p1 @ p2 #65410

Merged
merged 5 commits into from
Oct 15, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/libsyntax/parse/parser/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?
Expand All @@ -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<Pat>) -> PResult<'a, P<Pat>> {
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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pnkfelix This code should be removed when #65490 is implemented / stabilized.

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")
Centril marked this conversation as resolved.
Show resolved Hide resolved
.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();
} 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`")
Centril marked this conversation as resolved.
Show resolved Hide resolved
.emit();
}

rhs.span = sp;
Ok(rhs)
}

/// Ban a range pattern if it has an ambiguous interpretation.
fn ban_pat_range_if_ambiguous(&self, pat: &Pat) -> PResult<'a, ()> {
match pat.kind {
Expand Down
40 changes: 40 additions & 0 deletions src/test/ui/parser/intersection-patterns.rs
Original file line number Diff line number Diff line change
@@ -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<u8> = 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
_ => {}
}
}
33 changes: 33 additions & 0 deletions src/test/ui/parser/intersection-patterns.stderr
Original file line number Diff line number Diff line change
@@ -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)`
Centril marked this conversation as resolved.
Show resolved Hide resolved

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`
Centril marked this conversation as resolved.
Show resolved Hide resolved

error: aborting due to 3 previous errors