SD-210 don't generate invalid forwarders under -Xgen-mixin-forwarders

With -Xgen-mixin-forwarders the compiler eagerly creates mixin
forwarders for methods inherited from traits, even if the JVM method
resolution would pick the correct default method.

When generating a such a forwarder for a Java-defined default method,
the mixin forwarder invokes the default method directly using
`invokespecial` (for Scala-defined trait methods, the forwarder uses
the static `m$` method that is generated for every trait method).

An `invokespecial` call is only legal if the named interface is a
direct parent of the current class. If this is not the case, we don't
generate the mixin forwarder and emit a warning.

In the tests there's also an example where a mixin forwarder is
required for correctness, but cannot be added because the corresponding
`invokespecial` call would be invalid. In this case we emit an error.
This is similar to what we already do for other super calls to Java-
defined default methods. The difference is that the super call is not
written by the user but generated by the mixin forwarder. The solution
is the same: add the interface as a direct parent.

1 files changed, 68 insertions, 1 deletions

diff --git a/test/junit/scala/lang/traits/BytecodeTest.scala b/test/junit/scala/lang/traits/BytecodeTest.scala
index e6c74b86ab..a92d2244a2 100644
--- a/test/junit/scala/lang/traits/BytecodeTest.scala
+++ b/test/junit/scala/lang/traits/BytecodeTest.scala
@@ -9,7 +9,6 @@ import scala.collection.JavaConverters._
 import scala.tools.asm.Opcodes
 import scala.tools.asm.Opcodes._
 import scala.tools.asm.tree.ClassNode
-import scala.tools.nsc.backend.jvm.opt.BytecodeUtils
 import scala.tools.partest.ASMConverters._
 import scala.tools.testing.BytecodeTesting
 import scala.tools.testing.BytecodeTesting._
@@ -259,6 +258,74 @@ class BytecodeTest extends BytecodeTesting {
     assertSameCode(getMethod(c, "m"),
       List(VarOp(ALOAD, 0), Invoke(INVOKESPECIAL, "A", "m", "()I", false), Op(IRETURN)))
   }
+
+  @Test
+  def sd210(): Unit = {
+    val forwardersCompiler = newCompiler(extraArgs = "-Xgen-mixin-forwarders")
+    val jCode = List("interface A { default int m() { return 1; } }" -> "A.java")
+
+
+    // used to crash in the backend (SD-210) under `-Xgen-mixin-forwarders`
+    val code1 =
+      """trait B1 extends A // called "B1" not "B" due to scala-dev#214
+        |class C extends B1
+      """.stripMargin
+
+    val List(_, c1a) = compileClasses(code1, jCode)
+    assert(getAsmMethods(c1a, "m").isEmpty) // ok, no forwarder
+
+    // here we test a warning. without `-Xgen-mixin-forwarders`, the forwarder would not be
+    // generated, it is not necessary for correctness.
+    val warn = "Unable to implement a mixin forwarder for method m in class C unless interface A is directly extended by class C"
+    val List(_, c1b) = forwardersCompiler.compileClasses(code1, jCode, allowMessage = _.msg.contains(warn))
+    assert(getAsmMethods(c1a, "m").isEmpty) // no forwarder
+
+
+    val code2 =
+      """abstract class B { def m(): Int }
+        |trait T extends B with A
+        |class C extends T
+      """.stripMargin
+
+    // here we test a compilation error. the forwarder is required for correctness, but it cannot be generated.
+    val err = "Unable to implement a mixin forwarder for method m in class C unless interface A is directly extended by class C"
+    val cs = compileClasses(code2, jCode, allowMessage = _.msg contains err)
+    assert(cs.isEmpty, cs.map(_.name))
+
+
+    val code3 =
+      """abstract class B { def m: Int }
+        |class C extends B with A
+      """.stripMargin
+
+    val List(_, c3) = compileClasses(code3, jCode)
+    // invokespecial to A.m is correct here: A is an interface, so resolution starts at A.
+    // https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.invokespecial
+    val ins3 = getMethod(c3, "m").instructions
+    assert(ins3 contains Invoke(INVOKESPECIAL, "A", "m", "()I", true), ins3.stringLines)
+
+
+    val code4 =
+      """trait B { self: A => override def m = 2 }
+        |class C extends A with B // forwarder, invokestatic B.m$
+      """.stripMargin
+
+    val List(_, c4) = compileClasses(code4, jCode)
+    val ins4 = getMethod(c4, "m").instructions
+    assert(ins4 contains Invoke(INVOKESTATIC, "B", "m$", "(LB;)I", true), ins4.stringLines)
+
+
+    // scala-only example
+    val code5 =
+      """trait AS { def m = 1 }
+        |abstract class B { def m: Int }
+        |class C extends B with AS // forwarder, invokestatic AS.m$
+      """.stripMargin
+
+    val List(_, _, c5) = compileClasses(code5)
+    val ins5 = getMethod(c5, "m").instructions
+    assert(ins5 contains Invoke(INVOKESTATIC, "AS", "m$", "(LAS;)I", true), ins5.stringLines)
+  }
 }
 
 object invocationReceiversTestCode {