Scala2Unpickler: Don't trust the owner field of tparams #12660
Merged
Conversation
This file contains hidden or 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
When a type parameter of a class external to the current pickle is referenced, Scala 2 will sometimes pickle it as if it was a type parameter of the current class. When we unpickle such a type parameter on our side, we enter it in the class, this leads the compiler to believe the class has more type parameters than it actually has. This doesn't happen in Scala 2 itself because it never enters unpickled type parameters in the class that owns them (it must be recreating them in some way, but I don't know how). It would be interesting to dig deeper to understand exactly how Scala 2 handles type parameter unpickling so we could emulate it given that this is not the first time we run into trouble here (cf scala#12120), but for now we fix the problem with the following heuristic: once we've loaded all the type parameters of a class (which we do eagerly via `ClassUnpickler#init()`), we simply stop from being entered any subsequent type parameter we see for this class. Time will tell if this is good enough. While I was touching that code, I also made ClassCompleter#completerTypeParams idempotent as one would expect it to be. Fixes scala#12641.
smarter
force-pushed
the
scala2-tparam-wrong-owner
branch
from
302c5f9 to
7535dd9
Compare
odersky
approved these changes
Jun 2, 2021
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.
When a type parameter of a class external to the current pickle is
referenced, Scala 2 will sometimes pickle it as if it was a type
parameter of the current class. When we unpickle such a type parameter
on our side, we enter it in the class, this leads the compiler to
believe the class has more type parameters than it actually has. This
doesn't happen in Scala 2 itself because it never enters unpickled type
parameters in the class that owns them (it must be recreating them in
some way, but I don't know how).
It would be interesting to dig deeper to understand exactly how Scala 2
handles type parameter unpickling so we could emulate it given that this
is not the first time we run into trouble here (cf #12120), but for now
we fix the problem with the following heuristic: once we've loaded all
the type parameters of a class (which we do eagerly via
ClassUnpickler#init()), we simply stop from being entered anysubsequent type parameter we see for this class. Time will tell if this
is good enough.
While I was touching that code, I also made
ClassCompleter#completerTypeParams idempotent as one would expect it to
be.
Fixes #12641.