Skip to content

-Xgen-mixin-forwarders crashes for default methods #210

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

Closed
lrytz opened this issue Aug 29, 2016 · 4 comments
Closed

-Xgen-mixin-forwarders crashes for default methods #210

lrytz opened this issue Aug 29, 2016 · 4 comments
Assignees
Milestone

Comments

@lrytz
Copy link
Member

lrytz commented Aug 29, 2016

trait O[T] extends java.util.Comparator[T] // Comparator has a method `default Comparator<T> reversed`

class C extends O[String] {
  def compare(a: String, b: String) = 0
}

crashes the compiler with -Xgen-mixin-forwarders:

java.lang.AssertionError: assertion failed:
  cannot invokespecial java/util/Comparator.reversed, the interface is not a direct parent.

        at scala.tools.nsc.Global.assert(Global.scala:229)
        at scala.tools.nsc.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genCallMethod(BCodeBodyBuilder.scala:1089)
        at scala.tools.nsc.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genApply(BCodeBodyBuilder.scala:584)
        at scala.tools.nsc.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genLoad(BCodeBodyBuilder.scala:298)
        at scala.tools.nsc.backend.jvm.BCodeSkelBuilder$PlainSkelBuilder.emitNormalMethodBody$1(BCodeSkelBuilder.scala:604)
        at scala.tools.nsc.backend.jvm.BCodeSkelBuilder$PlainSkelBuilder.genDefDef(BCodeSkelBuilder.scala:636)
@lrytz
Copy link
Member Author

lrytz commented Aug 29, 2016

maybe something to fix for RC1..?

@lrytz
Copy link
Member Author

lrytz commented Aug 29, 2016

For reference: in M4 we add the interface as a direct parent during code gen when emitting a super call to a default method: https://github.com/scala/scala/blob/v2.12.0-M4/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala#L1083-L1084

In a later commit this information ("late interfaces") was also encoded in the ClassBType: scala/scala@3fca034

All of this was removed (for M5) when we changed the encoding to emit static methods for trait methods, super calls would directly target the static method (scala/scala@7d51b3f). A super call to a java-defined default method is now only allowed if the interface is a direct parent, this is checked during Mixins (scala/scala@7d51b3f#diff-8b2a191e6261b31a9a6462e4a01d3b9cR351).

The -Xgen-mixin-forwarders flag was added in (scala/scala@a0590aa), and it does its job also for inherited default methods. If the interface is not a direct parent of the class, an assertion in the backend will trigger the crash.

@lrytz lrytz added this to the 2.12.0-RC1 milestone Aug 30, 2016
@lrytz lrytz self-assigned this Aug 30, 2016
@lrytz
Copy link
Member Author

lrytz commented Aug 30, 2016

Here's an example that crashes without the -Xgen-mixin-forwarders flag:

A.java:

interface A {
  default int m() { return 1; }
}

Test.scala

class B extends A { override def m = 2 }
trait T extends A
class C extends B with T {
  override def m = super[T].m // should invoke A.m
}

The super[T].m invocation is translated to invokespecial A.m. This causes the assertion error in the backend: A is not a direct parent of C.

To translate the static binding super[T].m, the compiler checks the owner of T.m, which is A. If A was a trait (written in Scala), the call would be translated to invokestatic A.m$. But the static method doesn't exist (A is written in Java), so the call is translated to invokespecial A.m.

This example is the same as in #143 (comment) but with of a Java-defined interface A instead of a superclass A. In this case, there also isn't a static method A.m$, so the call is translated to invokespecial A.m, which does not crash the backend (A is a class, the invokespecial is valid), but will resolve to B.m at runtime.

@lrytz
Copy link
Member Author

lrytz commented Aug 30, 2016

scala/scala#5369

@adriaanm adriaanm closed this as completed Sep 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants