Fix yet another parser infinite loop
This commit is an example of fixing a common parser error: infinite
loop due to error recovery.
This error typically happens when we parse a list of items and fail to
parse a specific item at the current position.
One choices is to skip a token and try to parse a list item at the
next position. This is a good, but not universal, default. When
parsing a list of arguments in a function call, you, for example,
don't want to skip over `fn`, because it's most likely that it is a
function declaration, and not a mistyped arg:
```
fn foo() {
quux(1, 2
fn bar() {
}
```
Another choice is to bail out of the loop immediately, but it isn't
perfect either: sometimes skipping over garbage helps:
```
quux(1, foo:, 92) // should skip over `:`, b/c that's part of `foo::bar`
```
In general, parser tries to balance these two cases, though we don't
have a definitive strategy yet.
However, if the parser accidentally neither skips over a token, nor
breaks out of the loop, then it becomes stuck in the loop infinitely
(there's an internal counter to self-check this situation and panic
though), and that's exactly what is demonstrated by the test.
To fix such situation, first of all, add the test case to tests/data/parser/{err,fuzz-failures}.
Then, run
```
RUST_BACKTRACE=short cargo test --package libsyntax2
````
to verify that parser indeed panics, and to get an idea what grammar
production is the culprit (look for `_list` functions!).
In this case, I see
```
10: libsyntax2::grammar::expressions::atom::match_arm_list
at crates/libsyntax2/src/grammar/expressions/atom.rs:309
```
and that's look like it might be a culprit. I verify it by adding
`eprintln!("loopy {:?}", p.current());` and indeed I see that this is
printed repeatedly.
Diagnosing this a bit shows that the problem is that
`pattern::pattern` function does not consume anything if the next
token is `let`. That is a good default to make cases like
```
let
let foo = 92;
```
where the user hasn't typed the pattern yet, to parse in a reasonable
they correctly.
For match arms, pretty much the single thing we expect is a pattern,
so, for a fix, I introduce a special variant of pattern that does not
do recovery.
This commit is contained in:
@@ -323,11 +323,9 @@ fn match_arm_list(p: &mut Parser) {
|
|||||||
// }
|
// }
|
||||||
fn match_arm(p: &mut Parser) -> BlockLike {
|
fn match_arm(p: &mut Parser) -> BlockLike {
|
||||||
let m = p.start();
|
let m = p.start();
|
||||||
loop {
|
patterns::pattern_r(p, TokenSet::EMPTY);
|
||||||
|
while p.eat(PIPE) {
|
||||||
patterns::pattern(p);
|
patterns::pattern(p);
|
||||||
if !p.eat(PIPE) {
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
if p.eat(IF_KW) {
|
if p.eat(IF_KW) {
|
||||||
expr_no_struct(p);
|
expr_no_struct(p);
|
||||||
|
|||||||
@@ -8,7 +8,11 @@ pub(super) const PATTERN_FIRST: TokenSet =
|
|||||||
];
|
];
|
||||||
|
|
||||||
pub(super) fn pattern(p: &mut Parser) {
|
pub(super) fn pattern(p: &mut Parser) {
|
||||||
if let Some(lhs) = atom_pat(p) {
|
pattern_r(p, PAT_RECOVERY_SET)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(super) fn pattern_r(p: &mut Parser, recovery_set: TokenSet) {
|
||||||
|
if let Some(lhs) = atom_pat(p, recovery_set) {
|
||||||
// test range_pat
|
// test range_pat
|
||||||
// fn main() {
|
// fn main() {
|
||||||
// match 92 { 0 ... 100 => () }
|
// match 92 { 0 ... 100 => () }
|
||||||
@@ -16,7 +20,7 @@ pub(super) fn pattern(p: &mut Parser) {
|
|||||||
if p.at(DOTDOTDOT) {
|
if p.at(DOTDOTDOT) {
|
||||||
let m = lhs.precede(p);
|
let m = lhs.precede(p);
|
||||||
p.bump();
|
p.bump();
|
||||||
atom_pat(p);
|
atom_pat(p, recovery_set);
|
||||||
m.complete(p, RANGE_PAT);
|
m.complete(p, RANGE_PAT);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -26,7 +30,7 @@ const PAT_RECOVERY_SET: TokenSet =
|
|||||||
token_set![LET_KW, IF_KW, WHILE_KW, LOOP_KW, MATCH_KW, R_PAREN, COMMA];
|
token_set![LET_KW, IF_KW, WHILE_KW, LOOP_KW, MATCH_KW, R_PAREN, COMMA];
|
||||||
|
|
||||||
|
|
||||||
fn atom_pat(p: &mut Parser) -> Option<CompletedMarker> {
|
fn atom_pat(p: &mut Parser, recovery_set: TokenSet) -> Option<CompletedMarker> {
|
||||||
let la0 = p.nth(0);
|
let la0 = p.nth(0);
|
||||||
let la1 = p.nth(1);
|
let la1 = p.nth(1);
|
||||||
if la0 == REF_KW || la0 == MUT_KW
|
if la0 == REF_KW || la0 == MUT_KW
|
||||||
@@ -56,7 +60,7 @@ fn atom_pat(p: &mut Parser) -> Option<CompletedMarker> {
|
|||||||
L_PAREN => tuple_pat(p),
|
L_PAREN => tuple_pat(p),
|
||||||
L_BRACK => slice_pat(p),
|
L_BRACK => slice_pat(p),
|
||||||
_ => {
|
_ => {
|
||||||
p.err_recover("expected pattern", PAT_RECOVERY_SET);
|
p.err_recover("expected pattern", recovery_set);
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user