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

NestedContext: move all contexts to parser #3270

Merged
merged 1 commit into from
Jul 24, 2023
Merged

Conversation

kitbellew
Copy link
Contributor

The current implementation was mistakenly sharing context with any other instantiated parsers, which would cause problems in a multi-threaded use such as within scalafmt. Fixes #3063.

The current implementation was mistakenly sharing context with any other
instantiated parsers, which would cause problems in a multi-threaded use
such as within scalafmt.
@kitbellew kitbellew requested a review from tgodzik July 23, 2023 21:23
@tgodzik
Copy link
Collaborator

tgodzik commented Jul 24, 2023

Interesting, do you think this was indeed the cause of #3063?

@kitbellew
Copy link
Contributor Author

Interesting, do you think this was indeed the cause of #3063?

i checked out one of repos mentioned in that ticket and ran the formatter several dozen times. not sbt, which caches results and doesn't always rerun, but the cli.

sometimes no error, sometimes multiple errors, and always in random files, and almost always in macros. since the error now includes the code and position, the error would never really make sense.

so it seemed a thread-safety error. these contexts are definitely a bug. after applying this change, i reran the format on the same repo many times again, and not a single error.

so, it's possible that this is the fix 😀

@kitbellew kitbellew merged commit 949255c into scalameta:main Jul 24, 2023
24 checks passed
@kitbellew kitbellew deleted the 3063 branch July 24, 2023 17:26
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.

Wrong pattern must be a value
2 participants