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

Rewindable parser #17

Merged
merged 12 commits into from
Oct 6, 2023
Merged

Rewindable parser #17

merged 12 commits into from
Oct 6, 2023

Conversation

nberth
Copy link
Collaborator

@nberth nberth commented Sep 22, 2023

These changes add the ability to restart parsing only a few tokens before any document change. This should greatly improve performance, especially when editing the middle or end of large files.

Changes also include some refactoring of the parser and pre-processor, with a unification of their interface (notably, with options always passed using a single record).

Some tests of the rewinding feature come as well, along with fixes for some bugs they helped discover ;-).

@CLAassistant
Copy link

CLAassistant commented Sep 22, 2023

CLA assistant check
All committers have signed the CLA.

nberth added 3 commits October 2, 2023 11:19
This commit adds the ability to restart parsing only a few tokens
before any document change.  This should greatly improve performance,
especially when editing the middle or end of large files.
@nberth nberth force-pushed the rewindable-parser branch 2 times, most recently from 454fcdb to 96c0a1c Compare October 3, 2023 12:42
nberth added 2 commits October 3, 2023 15:43
Additionally, use a dedicated token to avoid retokenization of program IDs
@nberth nberth force-pushed the rewindable-parser branch from 96c0a1c to 403917f Compare October 3, 2023 14:23
@nberth nberth marked this pull request as ready for review October 3, 2023 14:24
@nberth nberth force-pushed the rewindable-parser branch 2 times, most recently from 85fa05d to 967b058 Compare October 3, 2023 15:31
@nberth nberth force-pushed the rewindable-parser branch from 967b058 to e420514 Compare October 4, 2023 07:32
@nberth nberth force-pushed the rewindable-parser branch from a2441c8 to 261193d Compare October 5, 2023 12:27
Copy link
Member

@lefessan lefessan left a comment

Choose a reason for hiding this comment

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

I stopped when I found yet another GADT. I think COBOL is complex enough without adding OCaml complexity on top of it. Please, progressively remove GADTs instead of adding new ones...

src/lsp/cobol_ast/misc_descr.ml Show resolved Hide resolved
src/lsp/cobol_ast/misc_descr.ml Outdated Show resolved Hide resolved
src/lsp/cobol_ast/misc_descr.ml Show resolved Hide resolved
src/lsp/cobol_ast/misc_descr.ml Outdated Show resolved Hide resolved
src/lsp/cobol_parser/cobol_parser.ml Show resolved Hide resolved
src/lsp/cobol_parser/parser_outputs.ml Show resolved Hide resolved
src/lsp/cobol_parser/parser_outputs.ml Outdated Show resolved Hide resolved
Copy link
Contributor

@bclement-ocp bclement-ocp left a comment

Choose a reason for hiding this comment

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

This touches too much code that I don't know to be able to do a meaningful proper review, but what I could read seems reasonable (if verbose).

src/lsp/cobol_lsp/lsp_document.ml Outdated Show resolved Hide resolved
src/lsp/cobol_lsp/lsp_document.ml Outdated Show resolved Hide resolved
@nberth nberth force-pushed the rewindable-parser branch from ce6bf5c to 75119bd Compare October 5, 2023 14:53
@nberth nberth merged commit adc6398 into OCamlPro:master Oct 6, 2023
3 checks passed
@nberth nberth deleted the rewindable-parser branch October 6, 2023 14:28
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.

5 participants