Skip to content

Commit

Permalink
use fields in autotupling parse recovery
Browse files Browse the repository at this point in the history
We introduce a new `PartialPResult`, which is like `PResult`, except
instead of the `Err` value being just a `DiagnosticBuilder`, it's a
tuple whose first element is of the `Ok` type and whose second element
is the diagnostic-builder: this provides a way to return to callers any
useful parsing work we did before hitting the error (at the cost of
having to sprinkle some awkward `.map_err`s around to convert between
`PResult` and `PartialPResult`).

`PartialPResult` is then used for more thorough recovery when a user
omits the parens from what we presume ought to have been a tuple
pattern: after emitting an error for the erroneous pattern, we can
continue working with the destructured fields as if the user had
correctly written a tuple, potentially catching more errors in
subsequent code without the user needing to recompile.
  • Loading branch information
zackmdavis committed Feb 28, 2018
1 parent ed6679c commit d615130
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 47 deletions.
5 changes: 5 additions & 0 deletions src/libsyntax/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ use std::str;

pub type PResult<'a, T> = Result<T, DiagnosticBuilder<'a>>;

/// Like `PResult`, but the `Err` value is a tuple whose first value is a
/// "partial result" (of some useful work we did before hitting the error), and
/// whose second value is the error.
pub type PartialPResult<'a, T> = Result<T, (T, DiagnosticBuilder<'a>)>;

#[macro_use]
pub mod parser;

Expand Down
55 changes: 38 additions & 17 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ use parse::{new_sub_parser_from_file, ParseSess, Directory, DirectoryOwnership};
use util::parser::{AssocOp, Fixity};
use print::pprust;
use ptr::P;
use parse::PResult;
use parse::{PResult, PartialPResult};
use tokenstream::{self, Delimited, ThinTokenStream, TokenTree, TokenStream};
use symbol::{Symbol, keywords};
use util::ThinVec;
Expand Down Expand Up @@ -3485,7 +3485,7 @@ impl<'a> Parser<'a> {
}

fn parse_pat_tuple_elements(&mut self, unary_needs_comma: bool)
-> PResult<'a, (Vec<P<Pat>>, Option<usize>)> {
-> PartialPResult<'a, (Vec<P<Pat>>, Option<usize>)> {
let mut fields = vec![];
let mut ddpos = None;

Expand All @@ -3494,19 +3494,24 @@ impl<'a> Parser<'a> {
ddpos = Some(fields.len());
if self.eat(&token::Comma) {
// `..` needs to be followed by `)` or `, pat`, `..,)` is disallowed.
fields.push(self.parse_pat()?);
let field = self.parse_pat()
.map_err(|err| ((fields.clone(), ddpos), err))?;
fields.push(field);
}
} else if ddpos.is_some() && self.eat(&token::DotDot) {
// Emit a friendly error, ignore `..` and continue parsing
self.span_err(self.prev_span, "`..` can only be used once per \
tuple or tuple struct pattern");
} else {
fields.push(self.parse_pat()?);
let field = self.parse_pat()
.map_err(|err| ((fields.clone(), ddpos), err))?;
fields.push(field);
}

if !self.check(&token::CloseDelim(token::Paren)) ||
(unary_needs_comma && fields.len() == 1 && ddpos.is_none()) {
self.expect(&token::Comma)?;
self.expect(&token::Comma)
.map_err(|err| ((fields.clone(), ddpos), err))?;
}
}

Expand Down Expand Up @@ -3690,11 +3695,11 @@ impl<'a> Parser<'a> {
}))
}

/// A wrapper around `parse_pat` with some special error handling for the
/// "top-level" patterns in a match arm, `for` loop, `let`, &c. (in contast
/// to subpatterns within such).
/// A wrapper around `parse_pat` with some special error handling and
/// recovery for the "top-level" patterns in a match arm, `for` loop,
/// `let`, &c. (in contast to subpatterns within such).
pub fn parse_top_level_pat(&mut self) -> PResult<'a, P<Pat>> {
let pat = self.parse_pat()?;
let mut pat = self.parse_pat()?;
if self.token == token::Comma {
// An unexpected comma after a top-level pattern is a clue that the
// user (perhaps more accustomed to some other language) forgot the
Expand All @@ -3703,12 +3708,21 @@ impl<'a> Parser<'a> {
// later.
let comma_span = self.span;
self.bump();
if let Err(mut err) = self.parse_pat_tuple_elements(false) {
// We didn't expect this to work anyway; we just wanted
// to advance to the end of the comma-sequence so we know
// the span to suggest parenthesizing
err.cancel();
}
let mut fields = vec![pat.clone()];
let (rest_fields, ddpos) = match self.parse_pat_tuple_elements(false) {
Ok(fields_ddpos) => fields_ddpos,
Err((fields_ddpos, mut err)) => {
// We didn't really expect this to work anyway; we want the
// partial result (so that post-recovery code knows about
// any bindings) and to advance to the end of the
// comma-sequence (so we know the span to suggest
// parenthesizing), but we'll emit our own error in just a
// moment
err.cancel();
fields_ddpos
}
};
fields.extend(rest_fields);
let seq_span = pat.span.to(self.prev_span);
let mut err = self.struct_span_err(comma_span,
"unexpected `,` in pattern");
Expand All @@ -3717,6 +3731,11 @@ impl<'a> Parser<'a> {
format!("({})", seq_snippet));
}
err.emit();
// Now that we've emitted our own error, the rest of the parser
// can pretend this was actually a tuple
pat = P(Pat { node: PatKind::Tuple(fields, ddpos),
span: seq_span,
id: ast::DUMMY_NODE_ID });
}
Ok(pat)
}
Expand Down Expand Up @@ -3746,7 +3765,8 @@ impl<'a> Parser<'a> {
token::OpenDelim(token::Paren) => {
// Parse (pat,pat,pat,...) as tuple pattern
self.bump();
let (fields, ddpos) = self.parse_pat_tuple_elements(true)?;
let (fields, ddpos) = self.parse_pat_tuple_elements(true)
.map_err(|(_partial, err)| err)?;
self.expect(&token::CloseDelim(token::Paren))?;
pat = PatKind::Tuple(fields, ddpos);
}
Expand Down Expand Up @@ -3839,7 +3859,8 @@ impl<'a> Parser<'a> {
}
// Parse tuple struct or enum pattern
self.bump();
let (fields, ddpos) = self.parse_pat_tuple_elements(false)?;
let (fields, ddpos) = self.parse_pat_tuple_elements(false)
.map_err(|(_partial, err)| err)?;
self.expect(&token::CloseDelim(token::Paren))?;
pat = PatKind::TupleStruct(path, fields, ddpos)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,15 @@ struct Genome {
allosomes: (Allosome, Allosome)
}

fn three_slice_to_tuple<T>(slice: &[T]) -> (T, T, T) {
(slice[0], slice[1], slice[2])
}

fn find_start_codon(strand: &[Nucleotide]) -> Option<usize> {
let mut reading_frame = strand.windows(3);
while let b1, b2, b3 = reading_frame.next().expect("there should be a start codon") {
//~^ ERROR unexpected `,` in pattern
while let b1, b2, b3 = three_slice_to_tuple(reading_frame.next()
//~^ ERROR unexpected `,` in pattern
.expect("there should be a start codon")) {
// ...
}
None
Expand All @@ -54,11 +59,9 @@ fn find_start_codon(strand: &[Nucleotide]) -> Option<usize> {
fn find_thr(strand: &[Nucleotide]) -> Option<usize> {
let mut reading_frame = strand.windows(3);
let mut i = 0;
if let b1, b2, b3 = reading_frame.next().unwrap() {
if let b1, b2, b3 = three_slice_to_tuple(reading_frame.next().unwrap()) {
//~^ ERROR unexpected `,` in pattern
match (b1, b2, b3) {
//~^ ERROR cannot find value `b2` in this scope
//~| ERROR cannot find value `b3` in this scope
Nucleotide::Adenine, Nucleotide::Cytosine, _ => {
//~^ ERROR unexpected `,` in pattern
return Some(i);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,50 +1,38 @@
error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:47:17
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:51:17
|
47 | while let b1, b2, b3 = reading_frame.next().expect("there should be a start codon") {
51 | while let b1, b2, b3 = three_slice_to_tuple(reading_frame.next()
| --^------- help: try adding parentheses: `(b1, b2, b3)`

error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:57:14
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:62:14
|
57 | if let b1, b2, b3 = reading_frame.next().unwrap() {
62 | if let b1, b2, b3 = three_slice_to_tuple(reading_frame.next().unwrap()) {
| --^------- help: try adding parentheses: `(b1, b2, b3)`

error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:62:32
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:65:32
|
62 | Nucleotide::Adenine, Nucleotide::Cytosine, _ => {
65 | Nucleotide::Adenine, Nucleotide::Cytosine, _ => {
| -------------------^------------------------ help: try adding parentheses: `(Nucleotide::Adenine, Nucleotide::Cytosine, _)`

error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:74:10
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:77:10
|
74 | for x, _barr_body in women.iter().map(|woman| woman.allosomes.clone()) {
77 | for x, _barr_body in women.iter().map(|woman| woman.allosomes.clone()) {
| -^----------- help: try adding parentheses: `(x, _barr_body)`

error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:78:10
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:81:10
|
78 | for x, y @ Allosome::Y(_) in men.iter().map(|man| man.allosomes.clone()) {
81 | for x, y @ Allosome::Y(_) in men.iter().map(|man| man.allosomes.clone()) {
| -^------------------- help: try adding parentheses: `(x, y @ Allosome::Y(_))`

error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:86:14
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:89:14
|
86 | let women, men: (Vec<Genome>, Vec<Genome>) = genomes.iter().cloned()
89 | let women, men: (Vec<Genome>, Vec<Genome>) = genomes.iter().cloned()
| -----^---- help: try adding parentheses: `(women, men)`

error[E0425]: cannot find value `b2` in this scope
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:59:20
|
59 | match (b1, b2, b3) {
| ^^ did you mean `b1`?

error[E0425]: cannot find value `b3` in this scope
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:59:24
|
59 | match (b1, b2, b3) {
| ^^ did you mean `b1`?

error: aborting due to 8 previous errors
error: aborting due to 6 previous errors

0 comments on commit d615130

Please sign in to comment.