-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix bytecode generation for Single Abstract Method lambdas #11839
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
Conversation
Partially based on scala/scala#6087 Fixes scala#10068 Fixes scala#11676
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM, @hrhino might be interested in taking a look too.
val methodName = samMethod.javaSimpleName | ||
val samMethodType = asmMethodType(samMethod).toASMType | ||
val needsGenericBridge = samMethodType != instantiatedMethodType | ||
val bridgeMethods = atPhase(erasurePhase.prev){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can actually be run at erasurePhase
because when running at a certain phase we see the types as they are before the phase denotation transformer is run.
val bridgeMethods = atPhase(erasurePhase.prev){ | |
val bridgeMethods = atPhase(erasurePhase) { |
@@ -927,8 +927,10 @@ object Types { | |||
*/ | |||
final def possibleSamMethods(using Context): Seq[SingleDenotation] = { | |||
record("possibleSamMethods") | |||
abstractTermMembers.toList.filterConserve(m => | |||
!m.symbol.matchingMember(defn.ObjectType).exists && !m.symbol.isSuperAccessor) | |||
atPhaseNoLater(erasurePhase.prev) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here:
atPhaseNoLater(erasurePhase.prev) { | |
atPhaseNoLater(erasurePhase) { |
val Seq(unerasedSam) = lambdaType.possibleSamMethods | ||
// We're now in erasure so the alternatives will have erased types | ||
val Seq(erasedSam: MethodType) = unerasedSam.symbol.alternatives.map(_.info) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid this complication I suggest changing possibleSamMethods
to do .map(_.current)
at the end so the denotations always correspond to the current phase.
mt.toASMType | ||
val methodName = samMethod.javaSimpleName | ||
val samMethodType = asmMethodType(samMethod).toASMType | ||
val needsGenericBridge = samMethodType != instantiatedMethodType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest adding the same comment than in Scala 2 for context:
val needsGenericBridge = samMethodType != instantiatedMethodType | |
// scala/bug#10334: make sure that a lambda object for `T => U` has a method `apply(T)U`, not only the `(Object)Object` | |
// version. Using the lambda a structural type `{def apply(t: T): U}` causes a reflective lookup for this method. | |
val needsGenericBridge = samMethodType != instantiatedMethodType |
9f2868f
to
3372b2a
Compare
3372b2a
to
2ab6f5d
Compare
@smarter reworks done |
https://github.com/lampepfl/dotty/runs/2185729042?check_suite_focus=true |
Looks like it was stuck in zio, maybe it would benefit from a larger heap? Restarting for now. |
Partially based on scala/scala#6087
Fixes #10068
Fixes #11676