Set parents during parse, not bind #1251
Open
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.
When loading files, Program construction needs to look a file's imports and resolve them. This requires parent pointers as determining what to do depends on context like the resolution mode, if it's in a require, etc.
This is a problem because binding can happen concurrently on the same AST as the same time as file loading, since ASTs can be reused between Programs.
We've worked around this by setting parents early in the parser just on the imports (was done in strada too), yet that still isn't enough to make the code not race (#970, #1031, #1224).
A simple way to solve this problem is to just make the parser the one in charge of setting pointers, rather than doing it during bind. This is slower, since the binder is already walking the AST and so usually has thing cached. But, it is a lot simpler to reason about.
Below are benchmarks which show the change; the real time itself is the critical number, showing that while in the checker, we save a millisecond in bind, we pay some 4ms for it during parse, likely due to caching / walking overhead.
If this seems undesirable, there are other fixes that we could try:
Fixes #1224