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

Refactor BufferLocalStore related code #17959

Merged
merged 5 commits into from
Nov 7, 2024
Merged

Conversation

Martin521
Copy link
Contributor

@Martin521 Martin521 commented Nov 5, 2024

Description

This PR comprises two commits.

The first one is pure refactoring.
It extracts all the code related to LexBuffer.BufferLocalStore from 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.

@Martin521 Martin521 requested a review from a team as a code owner November 5, 2024 15:49
Copy link
Contributor

github-actions bot commented Nov 5, 2024

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@psfinaki psfinaki added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Nov 5, 2024
Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

Thank you Martin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants