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

Basic parser error recovery #380

Merged
merged 12 commits into from
Jun 12, 2023
Merged

Basic parser error recovery #380

merged 12 commits into from
Jun 12, 2023

Conversation

bamarsha
Copy link
Contributor

@bamarsha bamarsha commented Jun 10, 2023

Add support for error recovery to the scanner, as well as some primitive parsers for ease of use. Include some basic error recovery for items and statements.

@bamarsha bamarsha changed the title Parser error recovery Parser error recovery framework Jun 11, 2023
@bamarsha bamarsha changed the title Parser error recovery framework Basic parser error recovery Jun 11, 2023
@bamarsha bamarsha marked this pull request as ready for review June 11, 2023 21:20
@bamarsha bamarsha requested review from idavis and swernli as code owners June 11, 2023 21:20
@idavis
Copy link
Collaborator

idavis commented Jun 12, 2023

Does it make sense to add recovery for statements missing ; as part of this PR or as a follow up?

@bamarsha
Copy link
Contributor Author

Can you be more specific? This PR does recover from statements missing ;. It recovers by eating all following statements until it finds a ;, which is not the smartest way to recover, but does avoid breaking the whole file. Do you mean you think the recovery should be smarter in this PR?

@idavis
Copy link
Collaborator

idavis commented Jun 12, 2023

Yes, smarter. Resume parsing as if ; had been there. Change the range of the error to where the ; should have been. The current impl has the error span on the next statement start.

@bamarsha
Copy link
Contributor Author

The diagnostic error spans are unchanged by this PR, it's the same as on main.

I think there are enough changes for infrastructure and refactoring in this PR that I don't want to spend too much time on the recovery behavior itself. I just did basic recovery for items and statements as a proof of concept for the recovery API. There's a long tail of recovery behavior improvements that we can do in follow-up PRs.

@idavis
Copy link
Collaborator

idavis commented Jun 12, 2023

Sounds good. I'll finish reviewing the rest of the PR.

@bamarsha bamarsha merged commit 7167620 into main Jun 12, 2023
@bamarsha bamarsha deleted the samarsha/recover branch June 12, 2023 20:10
@minestarks minestarks mentioned this pull request Jun 13, 2023
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants