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

fix parsing table cache bug #17816

Merged
merged 4 commits into from
Oct 8, 2024

Conversation

Martin521
Copy link
Contributor

Description

This is a fix for a bug in Internal.Utilities.Text.Parsing.Implementation.AssocTable.
The cache algorithm in AssocTable.Read relies on unused cache locations being marked.
In the implementation of the algorithm in AssocTable, 0 was used as marker.
However, 0 is also a valid key. This fix changes the marker to -1.

(In the context of my "scoped nowarn" work, a new token type that I added was not parsed correctly. It took me some time to find the reason.)

@Martin521 Martin521 requested a review from a team as a code owner September 29, 2024 13:12
Copy link
Contributor

github-actions bot commented Sep 29, 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

@T-Gro
Copy link
Member

T-Gro commented Sep 30, 2024

The contents of the cache are semantically just pairs of numbers, right?
But since it predates existence of struct(key,value) tuples, it just uses adjacent array indexes to store the key+value.

?

@Martin521
Copy link
Contributor Author

The contents of the cache are semantically just pairs of numbers, right?
But since it predates existence of struct(key,value) tuples, it just uses adjacent array indexes to store the key+value.

Yes

@Martin521
Copy link
Contributor Author

Can you add the "no release notes required" label?

@edgarfgp edgarfgp added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Sep 30, 2024
@vzarytovskii vzarytovskii merged commit 2109d5d into dotnet:main Oct 8, 2024
32 checks passed
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.

4 participants