SD-182 compiler option -Xgen-mixin-forwarders

Introduce a compiler option -Xgen-mixin-forwarders to always generate
mixin forwarder methods.

3 files changed, 73 insertions, 35 deletions

diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
index 4d236b226d..dae8539c66 100644
--- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
@@ -133,6 +133,7 @@ trait ScalaSettings extends AbsScalaSettings
 
   val XnoPatmatAnalysis = BooleanSetting ("-Xno-patmat-analysis", "Don't perform exhaustivity/unreachability analysis. Also, ignore @switch annotation.")
   val XfullLubs         = BooleanSetting ("-Xfull-lubs", "Retains pre 2.10 behavior of less aggressive truncation of least upper bounds.")
+  val XgenMixinForwarders = BooleanSetting("-Xgen-mixin-forwarders", "Generate forwarder methods in classes inhering concrete methods from traits.")
 
   // XML parsing options
   object XxmlSettings extends MultiChoiceEnumeration {
diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index d62b77dac2..b787f64846 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -246,43 +246,51 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
           case NoSymbol =>
             val isMemberOfClazz = clazz.info.findMember(member.name, 0, 0L, stableOnly = false).alternatives.contains(member)
             if (isMemberOfClazz) {
-              // `member` is a concrete method defined in `mixinClass`, which is a base class of
-              // `clazz`, and the method is not overridden in `clazz`. A forwarder is needed if:
-              //
-              //   - A non-trait base class of `clazz` defines a matching method. Example:
-              //       class C {def f: Int}; trait T extends C {def f = 1}; class D extends T
-              //     Even if C.f is abstract, the forwarder in D is needed, otherwise the JVM would
-              //     resolve `D.f` to `C.f`, see jvms-6.5.invokevirtual.
-              //
-              //   - There exists another concrete, matching method in a parent interface `p` of
-              //     `clazz`, and the `mixinClass` does not itself extend `p`. In this case the
-              //     forwarder is needed to disambiguate. Example:
-              //       trait T1 {def f = 1}; trait T2 extends T1 {override def f = 2}; class C extends T2
-              //     In C we don't need a forwarder for f because T2 extends T1, so the JVM resolves
-              //     C.f to T2.f non-ambiguously. See jvms-5.4.3.3, "maximally-specific method".
-              //       trait U1 {def f = 1}; trait U2 {self:U1 => override def f = 2}; class D extends U2
-              //     In D the forwarder is needed, the interfaces U1 and U2 are unrelated at the JVM
-              //     level.
-
-              @tailrec
-              def existsCompetingMethod(baseClasses: List[Symbol]): Boolean = baseClasses match {
-                case baseClass :: rest =>
-                  if (baseClass ne mixinClass) {
-                    val m = member.overriddenSymbol(baseClass)
-                    val isCompeting = m.exists && {
-                      !m.owner.isTraitOrInterface ||
-                        (!m.isDeferred && !mixinClass.isNonBottomSubClass(m.owner))
-                    }
-                    isCompeting || existsCompetingMethod(rest)
-                  } else existsCompetingMethod(rest)
-
-                case _ => false
+              def genForwarder(): Unit = {
+                cloneAndAddMixinMember(mixinClass, member).asInstanceOf[TermSymbol] setAlias member
               }
 
-              if (existsCompetingMethod(clazz.baseClasses))
-                cloneAndAddMixinMember(mixinClass, member).asInstanceOf[TermSymbol] setAlias member
-              else if (!settings.nowarnDefaultJunitMethods && JUnitTestClass.exists && member.hasAnnotation(JUnitTestClass))
-                warning(member.pos, "JUnit tests in traits that are compiled as default methods are not executed by JUnit 4. JUnit 5 will fix this issue.")
+              if (settings.XgenMixinForwarders) genForwarder()
+              else {
+
+                // `member` is a concrete method defined in `mixinClass`, which is a base class of
+                // `clazz`, and the method is not overridden in `clazz`. A forwarder is needed if:
+                //
+                //   - A non-trait base class of `clazz` defines a matching method. Example:
+                //       class C {def f: Int}; trait T extends C {def f = 1}; class D extends T
+                //     Even if C.f is abstract, the forwarder in D is needed, otherwise the JVM would
+                //     resolve `D.f` to `C.f`, see jvms-6.5.invokevirtual.
+                //
+                //   - There exists another concrete, matching method in a parent interface `p` of
+                //     `clazz`, and the `mixinClass` does not itself extend `p`. In this case the
+                //     forwarder is needed to disambiguate. Example:
+                //       trait T1 {def f = 1}; trait T2 extends T1 {override def f = 2}; class C extends T2
+                //     In C we don't need a forwarder for f because T2 extends T1, so the JVM resolves
+                //     C.f to T2.f non-ambiguously. See jvms-5.4.3.3, "maximally-specific method".
+                //       trait U1 {def f = 1}; trait U2 {self:U1 => override def f = 2}; class D extends U2
+                //     In D the forwarder is needed, the interfaces U1 and U2 are unrelated at the JVM
+                //     level.
+
+                @tailrec
+                def existsCompetingMethod(baseClasses: List[Symbol]): Boolean = baseClasses match {
+                  case baseClass :: rest =>
+                    if (baseClass ne mixinClass) {
+                      val m = member.overriddenSymbol(baseClass)
+                      val isCompeting = m.exists && {
+                        !m.owner.isTraitOrInterface ||
+                          (!m.isDeferred && !mixinClass.isNonBottomSubClass(m.owner))
+                      }
+                      isCompeting || existsCompetingMethod(rest)
+                    } else existsCompetingMethod(rest)
+
+                  case _ => false
+                }
+
+                if (existsCompetingMethod(clazz.baseClasses))
+                  genForwarder()
+                else if (!settings.nowarnDefaultJunitMethods && JUnitTestClass.exists && member.hasAnnotation(JUnitTestClass))
+                  warning(member.pos, "JUnit tests in traits that are compiled as default methods are not executed by JUnit 4. JUnit 5 will fix this issue.")
+              }
             }
 
           case _        =>
diff --git a/test/junit/scala/lang/traits/BytecodeTest.scala b/test/junit/scala/lang/traits/BytecodeTest.scala
index ec8508df99..e6c74b86ab 100644
--- a/test/junit/scala/lang/traits/BytecodeTest.scala
+++ b/test/junit/scala/lang/traits/BytecodeTest.scala
@@ -230,6 +230,35 @@ class BytecodeTest extends BytecodeTesting {
       List(ALOAD, ILOAD, PUTFIELD, ALOAD, ACONST_NULL, "<init>", RETURN))
   }
 
+  @Test
+  def mixinForwarders(): Unit = {
+    val code =
+      """trait T { def f = 1 }
+        |class C extends T
+      """.stripMargin
+    val List(c1, _) = compileClasses(code)
+    val List(c2, _) = newCompiler(extraArgs = "-Xgen-mixin-forwarders").compileClasses(code)
+    assert(getMethods(c1, "f").isEmpty)
+    assertSameCode(getMethod(c2, "f"),
+      List(VarOp(ALOAD, 0), Invoke(INVOKESTATIC, "T", "f$", "(LT;)I", true), Op(IRETURN)))
+  }
+
+  @Test
+  def sd143(): Unit = {
+    // this tests the status quo, which is wrong.
+    val code =
+      """class A { def m = 1 }
+        |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
+        |}
+      """.stripMargin
+    val List(_, _, c, _) = compileClasses(code)
+    // even though the bytecode refers to A.m, invokespecial will resolve to B.m
+    assertSameCode(getMethod(c, "m"),
+      List(VarOp(ALOAD, 0), Invoke(INVOKESPECIAL, "A", "m", "()I", false), Op(IRETURN)))
+  }
 }
 
 object invocationReceiversTestCode {