Refactor BufferLocalStore related code #17959
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
psfinaki
added
the
NO_RELEASE_NOTES
T-Gro
reviewed
Nov 7, 2024
T-Gro
approved these changes
Nov 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR comprises two commits.
The first one is pure refactoring.
It extracts all the code related to
LexBuffer.BufferLocalStorefrom ParseHelpers.fs and moves it to a separate file LexerStore.fs.Some common code is factored into separate functions (at the top of the file). Some names are simplified.
In consuming code, open statements and names are adapted accordingly.
This refactoring, I believe, makes the use of BufferLocalStore easier to understand, track and maintain.
The separation into LexerStore.fs makes also sense because all the code is similar in that it collects information during lexing for later "meta" use (like editor support, compiler configuration, documentation.), and there is no direct relation to the rest of the code in ParseHelpers.fs.
It is part of my work on "scoped nowarn" (that will also need to make use of BufferLocalStore), but separated out.
The second commit removes resetting the store in
LexHelp.reusingLexbufForParsing.I found that all code paths leading to this place come with a freshly created Lexbuf, i.e. empty BufferLocalStore.