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

Block on parse error #1002

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Block on parse error #1002

wants to merge 4 commits into from

Conversation

rtetley
Copy link
Collaborator

@rtetley rtetley commented Jan 21, 2025

At the moment the mechanism for parse errors are different than exec errors.
This is a limitation as we cannot block on the first parse error.
We introduce a mechanism to record a sentence state allowing us to detect that a sentence has a parse error and prevent from executing it.
Closes #877

Instead of ignoring parse errors, we make the ast a sentence_state object
which either contains the parse_error or the parsed_ast.
This will allow to detect parsing errors and block on the error instead
of checking through it.
log @@ "handling parse error at " ^ string_of_int start;
let stop = Stream.count stream in
let parsing_error = { msg; start; stop; qf} in
let synterp_state = Vernacstate.Synterp.freeze () in
Copy link
Member

Choose a reason for hiding this comment

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

I think you should just pass the synterp state you used to parser the sentence, rather than freezing it here.
I think in practice it will be the same, but there is no need to recompute the snapshot.

We create a Block type in the scheduler (and its corresponding PBlock
in execution manager) which will send an exec_error if one tries to execute
through a parse error. If the user has set block_on_error mode then the behaviour
is now identical with parsing errors and execution errors.
@rtetley rtetley force-pushed the block-on-parse-error branch from 61f4d97 to e4016bf Compare January 23, 2025 13:38
@rtetley rtetley marked this pull request as ready for review January 23, 2025 13:40
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.

Checking can "work" even if parsing fails
2 participants