Fix stale symbol crashes in some path depended types in macro contexts #18077
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
The reason for the stale symbols was the fact that they were derived from the previous (suspended) run (by calling a method from that run), and path depended types seem to use an optimized path utilizing initial designator, unlike the other kinds of types, which seem to either not need calculating denotation or the denotation there is able to update it's own validity. Since that is impossible to do for those path dependent types, if they are invalid in the run recaluclate the Symbol, otherwise use `currentSymbol` as before.
To fix we recalculate the symbol from directly from NamedType when from invalid run, instead of updating it's validity (which is impossible, as its owners do not include it in their decls).
|
Hello, I commented in #17294 that I encountered a similar problem but was unable to minimize it. The code does not involve macros, but involves compiler suspension/resume. Compiled code behaves as expected. In a nutshell:
|
dwijnand
reviewed
Jun 30, 2023
|
OMG, thank you so much for solving this! It has plagued my library in so many ways. |
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.
Closes #17152
Closes #17294
In general the issues stemmed from the fact that after suspending runs due to the found macros, when typing Ident of the called method, symbols from the previous run are found there. Some of them either are able to have their validity updated while returning the denotation (which those path dependent types are unable to do, since their owners now have updated decls, which do not include the stale symbol), or do not need denotation as part of a code path they rely on.
The fixes simply check if the to-be-used symbol has a valid runID, and if not then recomputes it.
The first commit fixes the minimizations from above GitHub issues. Both minimizations by design have to result in cyclic macro errors (which they now do), so they were placed in
neg-macrostests.By some experimentation, I ended up with another, slightly different stale symbol crash, with the stale symbol trying to create a denotation in different place (in
withPrefix), requiring an additional fix there, included in the second commit. This minimization, unlike the previous ones, does compile successfully, without cyclic macro errors, which shows the issue was not exclusive to those.