Handle pickled forward references in pickled expressions #16855
Merged
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
nicolasstucki
force-pushed
the
fix-16843
branch
from
661bc28
to
eb92c52
Compare
smarter
requested changes
Feb 9, 2023
Comment on lines
7
to
16
| val b = ${ | ||
| val a = '{ | ||
| (1: Int) match | ||
| case x @ (y: Int) => 0 | ||
| } | ||
| a | ||
| } | ||
|
|
||
| (1: Int) match | ||
| case x @ (y: Int) => 0 |
There was a problem hiding this comment.
The test case in #16843 is easier to understand/debug, so I would add it too even if we want to test more complex cases.
| current | ||
| // If current does not exist it is beacause this is a pickled expression | ||
| // and the owner is located outside the expression (i.e. pickled quote). | ||
| current.orElse(ctx.owner) |
There was a problem hiding this comment.
This could hide bugs if we're not in fact in a pickled quote. I think it'd be clearer if we stored the quote owner instead. Something like:
/** If this is a pickled quote, the owner of the quote, otherwise NoSymbol. */
private var rootOwner: Symbol = NoSymbol
...
def unpickle(mode: UnpickleMode)(using Context): List[Tree] = {
if mode != UnpickleMode.TopLevel then rootOwner = ctx.owner
...
}
...
class OwnerTree(val addr: Addr, tag: Int, reader: TreeReader, val end: Addr) {
...
try search(children, rootOwner)
...
}
We do not generate |
nicolasstucki
force-pushed
the
fix-16843
branch
from
eb92c52
to
3c74eec
Compare
To compute forward references it was assumed that the owner of that symbol can be found in the TASTy. This is not always the case with TASTy expressions of pickled quotes. The owner might be outside the quote, in this case the context already has the owner of the referenced symbol. These are local symbols defined at the top-level of the TASTy. Fixes scala#16843
nicolasstucki
force-pushed
the
fix-16843
branch
from
3c74eec
to
96bfb69
Compare
smarter
approved these changes
Feb 15, 2023
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
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.
To compute forward references it was assumed that the owner of that symbol can be found in the TASTy. This is not always the case with TASTy expressions of pickled quotes. The owner might be outside the quote, in this case the context already has the owner of the referenced symbol. These are local symbols defined at the top-level of the TASTy.
Fixes #16843