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

Add checkpoint logic for the parser #11441

Merged
merged 1 commit into from
May 17, 2024

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented May 16, 2024

Summary

This PR adds the checkpoint logic to the parser and token source. It also updates the lexer checkpoint to contain the error position.

@dhruvmanila dhruvmanila force-pushed the dhruv/lazy-lexer branch 4 times, most recently from 47ec6fe to dcab2ef Compare May 17, 2024 10:07
Base automatically changed from dhruv/lazy-lexer to dhruv/parser-phase-2 May 17, 2024 11:34
@dhruvmanila dhruvmanila marked this pull request as ready for review May 17, 2024 11:37
@dhruvmanila dhruvmanila merged commit 08f881b into dhruv/parser-phase-2 May 17, 2024
6 of 18 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/parser-checkpoint branch May 17, 2024 11:37
dhruvmanila added a commit that referenced this pull request May 20, 2024
## Summary

This PR adds the checkpoint logic to the parser and token source. It
also updates the lexer checkpoint to contain the error position.
dhruvmanila added a commit that referenced this pull request May 23, 2024
## Summary

This PR adds the checkpoint logic to the parser and token source. It
also updates the lexer checkpoint to contain the error position.
dhruvmanila added a commit that referenced this pull request May 23, 2024
## Summary

This PR adds the checkpoint logic to the parser and token source. It
also updates the lexer checkpoint to contain the error position.
dhruvmanila added a commit that referenced this pull request May 24, 2024
## Summary

This PR adds the checkpoint logic to the parser and token source. It
also updates the lexer checkpoint to contain the error position.
dhruvmanila added a commit that referenced this pull request May 27, 2024
## Summary

This PR adds the checkpoint logic to the parser and token source. It
also updates the lexer checkpoint to contain the error position.
Comment on lines +649 to +653
self.tokens.rewind(checkpoint.tokens);
self.errors.truncate(checkpoint.errors_position);
self.current_token_id = checkpoint.current_token_id;
self.prev_token_end = checkpoint.prev_token_end;
self.recovery_context = checkpoint.recovery_context;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I like to use destructuring here. It adds a compile time guarantee that all fields are used (at the cost of a bit more code)

let ParserCheckpoint { 
	tokens, 
	errors_position,
	prev_token_end,
	recovery_context
} = checkpoint; 

self.tokens.rewind(tokens);
self.errors.truncate(errors_position);
...

You can also do the destructuring right in the method signature, although it then becomes a bit hard to read.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I recently realized that with the amount of fields increasing in the lexer checkpoint and will be making the changes in a follow-up PR. Thanks for the recommendation.

dhruvmanila added a commit that referenced this pull request May 28, 2024
## Summary

This PR adds the checkpoint logic to the parser and token source. It
also updates the lexer checkpoint to contain the error position.
dhruvmanila added a commit that referenced this pull request May 30, 2024
## Summary

This PR adds the checkpoint logic to the parser and token source. It
also updates the lexer checkpoint to contain the error position.
dhruvmanila added a commit that referenced this pull request May 31, 2024
## Summary

This PR adds the checkpoint logic to the parser and token source. It
also updates the lexer checkpoint to contain the error position.
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.

None yet

2 participants