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

Fix fn front matter parsing ICE from invalid code. #60132

Merged
merged 1 commit into from Apr 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/libsyntax/parse/parser.rs
Expand Up @@ -6575,7 +6575,12 @@ impl<'a> Parser<'a> {
};
(respan(self.prev_span, Constness::NotConst), unsafety, abi)
};
self.expect_keyword(keywords::Fn)?;
if !self.eat_keyword(keywords::Fn) {
// It is possible for `expect_one_of` to recover given the contents of
// `self.expected_tokens`, therefore, do not use `self.unexpected()` which doesn't
// account for this.
if !self.expect_one_of(&[], &[])? { unreachable!() }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Although I think this change will fix the general problem today, I feel we could also address the side of the recovery code as well to not attempt to close the block when encountering an incorrect closing delimiter. It is tricky because what is best to do depends entirely on the type of mistake made and we don't do global analysis...

What do you think? Should we have a follow up PR/ticket with different common cases to test the recovery mechanism?

Copy link
Member Author

Choose a reason for hiding this comment

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

I’d land this as is to fix the ICE, but I think a follow up to try and handle this case more gracefully definitely makes sense.

I spent a bunch of time trying to work out a nice way to recover from this case but I couldn’t see one. If we don’t attempt to close the block when encountering an unclosed delimiter then that’ll regress other cases that we recover from currently, it’s a tough one.

Ok((constness, unsafety, asyncness, abi))
}

Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/issue-60075.rs
@@ -0,0 +1,12 @@
fn main() {}

trait T {
fn qux() -> Option<usize> {
let _ = if true {
});
//~^ ERROR expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `;`
//~^^ ERROR expected one of `.`, `;`, `?`, `else`, or an operator, found `}`
//~^^^ ERROR 6:11: 6:12: expected identifier, found `;`
//~^^^^ ERROR missing `fn`, `type`, or `const` for trait-item declaration
Some(4)
}
37 changes: 37 additions & 0 deletions src/test/ui/issue-60075.stderr
@@ -0,0 +1,37 @@
error: expected one of `.`, `;`, `?`, `else`, or an operator, found `}`
--> $DIR/issue-60075.rs:6:10
|
LL | });
| ^ expected one of `.`, `;`, `?`, `else`, or an operator here

error: expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `;`
--> $DIR/issue-60075.rs:6:11
|
LL | fn qux() -> Option<usize> {
| - unclosed delimiter
LL | let _ = if true {
LL | });
| ^
| |
| help: `}` may belong here

error: expected identifier, found `;`
--> $DIR/issue-60075.rs:6:11
|
LL | });
| ^ expected identifier

error: missing `fn`, `type`, or `const` for trait-item declaration
--> $DIR/issue-60075.rs:6:12
|
LL | });
| ____________^
LL | |
LL | |
LL | |
LL | |
LL | | Some(4)
| |________^ missing `fn`, `type`, or `const`

error: aborting due to 4 previous errors