SD-98 don't emit unnecessary mixin forwarders

In most cases when a class inherits a concrete method from a trait we
don't need to generate a forwarder to the default method in the class.

t5148 is moved to pos as it compiles without error now. the error
message ("missing or invalid dependency") is still tested by t6440b.

12 files changed, 223 insertions, 57 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index ed7ef0d8fd..03935c3d67 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -47,7 +47,6 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
        sym.isMethod
     && (!sym.hasFlag(DEFERRED | SUPERACCESSOR) || (sym hasFlag lateDEFERRED))
     && sym.owner.isTrait
-    && sym.isMethod
     && (!sym.isModule || sym.hasFlag(PRIVATE | LIFTED))
     && (!(sym hasFlag (ACCESSOR | SUPERACCESSOR)) || sym.isLazy)
     && !sym.isPrivate
@@ -240,8 +239,41 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
       for (member <- mixinClass.info.decls ; if isImplementedStatically(member)) {
         member overridingSymbol clazz match {
           case NoSymbol =>
-            if (clazz.info.findMember(member.name, 0, 0L, stableOnly = false).alternatives contains member)
-              cloneAndAddMixinMember(mixinClass, member).asInstanceOf[TermSymbol] setAlias member
+            val isMemberOfClazz = clazz.info.findMember(member.name, 0, 0L, stableOnly = false).alternatives.contains(member)
+            if (isMemberOfClazz) {
+              val competingMethods = clazz.baseClasses.iterator
+                .filter(_ ne mixinClass)
+                .map(member.overriddenSymbol)
+                .filter(_.exists)
+                .toList
+
+              // `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 defines 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 any of the base classes, and
+              //     the `mixinClass` does not itself extend that base class. 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.
+
+              lazy val mixinSuperInterfaces = mixinClass.ancestors.filter(_.isTraitOrInterface)
+              val needsForwarder = competingMethods.exists(m => {
+                !m.owner.isTraitOrInterface ||
+                  (!m.isDeferred && !mixinSuperInterfaces.contains(m.owner))
+              })
+              if (needsForwarder)
+                cloneAndAddMixinMember(mixinClass, member).asInstanceOf[TermSymbol] setAlias member
+            }
+
           case _        =>
         }
       }
diff --git a/test/files/neg/t4749.check b/test/files/neg/t4749.check
index 3539140954..6bd2550097 100644
--- a/test/files/neg/t4749.check
+++ b/test/files/neg/t4749.check
@@ -26,7 +26,7 @@ t4749.scala:26: warning: Fail6 has a main method with parameter type Array[Strin
   object Fail6 {
          ^
 t4749.scala:42: warning: Win3 has a main method with parameter type Array[String], but bippy.Win3 will not be a runnable program.
-  Reason: main method must have exact signature (Array[String])Unit
+  Reason: main methods cannot refer to type parameters or abstract types.
   object Win3 extends WinBippy[Unit] { }
          ^
 error: No warnings can be incurred under -Xfatal-warnings.
diff --git a/test/files/neg/t5148.check b/test/files/neg/t5148.check
deleted file mode 100644
index 1f58c235ce..0000000000
--- a/test/files/neg/t5148.check
+++ /dev/null
@@ -1,16 +0,0 @@
-error: missing or invalid dependency detected while loading class file 'Imports.class'.
-Could not access term memberHandlers in class scala.tools.nsc.interpreter.IMain,
-because it (or its dependencies) are missing. Check your build definition for
-missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.)
-A full rebuild may help if 'Imports.class' was compiled against an incompatible version of scala.tools.nsc.interpreter.IMain.
-error: missing or invalid dependency detected while loading class file 'Imports.class'.
-Could not access type Wrapper in class scala.tools.nsc.interpreter.IMain.Request,
-because it (or its dependencies) are missing. Check your build definition for
-missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.)
-A full rebuild may help if 'Imports.class' was compiled against an incompatible version of scala.tools.nsc.interpreter.IMain.Request.
-error: missing or invalid dependency detected while loading class file 'Imports.class'.
-Could not access type Request in class scala.tools.nsc.interpreter.IMain,
-because it (or its dependencies) are missing. Check your build definition for
-missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.)
-A full rebuild may help if 'Imports.class' was compiled against an incompatible version of scala.tools.nsc.interpreter.IMain.
-three errors found
diff --git a/test/files/neg/t5148.scala b/test/files/pos/t5148.scala
index fca64e57df..fca64e57df 100644
--- a/test/files/neg/t5148.scala
+++ b/test/files/pos/t5148.scala
diff --git a/test/files/run/mixin-signatures.check b/test/files/run/mixin-signatures.check
index 77bff79ac8..9961992e2d 100644
--- a/test/files/run/mixin-signatures.check
+++ b/test/files/run/mixin-signatures.check
@@ -1,19 +1,23 @@
 class Test$bar1$ {
-  public java.lang.String Test$bar1$.f(java.lang.Object)
+  public default java.lang.String Foo1.f(java.lang.Object)
+    generic: public default java.lang.String Foo1.f(T)
   public java.lang.Object Test$bar1$.f(java.lang.Object) <bridge> <synthetic>
   public java.lang.String Test$bar1$.g(java.lang.String)
   public java.lang.Object Test$bar1$.g(java.lang.Object) <bridge> <synthetic>
   public java.lang.String Test$bar1$.g(java.lang.Object) <bridge> <synthetic>
-  public java.lang.Object Test$bar1$.h(java.lang.Object)
+  public default java.lang.Object Base.h(java.lang.Object)
+    generic: public default R Base.h(T)
 }
 
 class Test$bar2$ {
-  public java.lang.Object Test$bar2$.f(java.lang.String)
+  public default java.lang.Object Foo2.f(java.lang.String)
+    generic: public default R Foo2.f(java.lang.String)
   public java.lang.Object Test$bar2$.f(java.lang.Object) <bridge> <synthetic>
   public java.lang.String Test$bar2$.g(java.lang.String)
   public java.lang.Object Test$bar2$.g(java.lang.Object) <bridge> <synthetic>
   public java.lang.Object Test$bar2$.g(java.lang.String) <bridge> <synthetic>
-  public java.lang.Object Test$bar2$.h(java.lang.Object)
+  public default java.lang.Object Base.h(java.lang.Object)
+    generic: public default R Base.h(T)
 }
 
 class Test$bar3$ {
@@ -23,7 +27,8 @@ class Test$bar3$ {
   public java.lang.String Test$bar3$.g(java.lang.String)
   public java.lang.Object Test$bar3$.g(java.lang.Object) <bridge> <synthetic>
   public java.lang.String Test$bar3$.g(java.lang.Object) <bridge> <synthetic>
-  public java.lang.Object Foo3.h(java.lang.Object)
+  public default java.lang.Object Base.h(java.lang.Object)
+    generic: public default R Base.h(T)
 }
 
 class Test$bar4$ {
@@ -33,7 +38,8 @@ class Test$bar4$ {
   public java.lang.String Test$bar4$.g(java.lang.String)
   public java.lang.Object Test$bar4$.g(java.lang.Object) <bridge> <synthetic>
   public java.lang.Object Test$bar4$.g(java.lang.String) <bridge> <synthetic>
-  public java.lang.Object Foo4.h(java.lang.Object)
+  public default java.lang.Object Base.h(java.lang.Object)
+    generic: public default R Base.h(T)
 }
 
 class Test$bar5$ {
@@ -45,7 +51,8 @@ class Test$bar5$ {
   public java.lang.Object Test$bar5$.g(java.lang.Object) <bridge> <synthetic>
   public java.lang.Object Test$bar5$.g(java.lang.String) <bridge> <synthetic>
   public java.lang.String Test$bar5$.g(java.lang.Object) <bridge> <synthetic>
-  public java.lang.Object Test$bar5$.h(java.lang.Object)
+  public default java.lang.Object Base.h(java.lang.Object)
+    generic: public default R Base.h(T)
 }
 
 interface Foo1 {
diff --git a/test/files/run/t5652.check b/test/files/run/t5652.check
index 326b22f9dd..a0fb6fe9b4 100644
--- a/test/files/run/t5652.check
+++ b/test/files/run/t5652.check
@@ -4,5 +4,4 @@ public default void T1.$init$()
 public final int A1.A1$$g$2()
 public int A1.f1()
 public final int A2.A2$$g$1()
-public int A2.f0()
 public int A2.f2()
diff --git a/test/files/run/t6554.scala b/test/files/run/t6554.scala
index 5d29d16666..eed139fea6 100644
--- a/test/files/run/t6554.scala
+++ b/test/files/run/t6554.scala
@@ -1,8 +1,14 @@
-trait Foo[A] {
+trait T1[A] {
   def minBy[B](b: B): A = ???
 }
- 
-class Bar extends Foo[Int]
+
+// The second trait is needed to make sure there's a forwarder generated in Bar.
+// otherwise Bar.minBy is just the inherited default method from T1.
+trait T2[A] { self: T1[A] =>
+  override def minBy[B](b: B): A = ???
+}
+
+class Bar extends T1[Int] with T2[Int]
 
 object Test extends App {
    val sigs = classOf[Bar].getDeclaredMethods.map(m => s"${m.toString} / ${m.toGenericString}").sorted
diff --git a/test/files/run/t7932.check b/test/files/run/t7932.check
index 3f0a0c4f62..a2ad84cd46 100644
--- a/test/files/run/t7932.check
+++ b/test/files/run/t7932.check
@@ -1,3 +1,6 @@
-warning: there was one feature warning; re-run with -feature for details
 public Category<?> C.category()
 public Category<scala.Tuple2> C.category1()
+public default Category<java.lang.Object> M1.category()
+public default Category<scala.Tuple2> M1.category1()
+public default Category<java.lang.Object> M2.category()
+public default Category<scala.Tuple2> M2.category1()
diff --git a/test/files/run/t7932.scala b/test/files/run/t7932.scala
index 8743abff88..e6bdbf2417 100644
--- a/test/files/run/t7932.scala
+++ b/test/files/run/t7932.scala
@@ -1,11 +1,28 @@
+import scala.language.higherKinds
+
 class Category[M[_, _]]
-trait M[F] {
+
+trait M1[F] {
   type X[a, b] = F
   def category: Category[X] = null
   def category1: Category[Tuple2] = null
 }
-abstract class C extends M[Float]
+
+// The second trait is needed to make sure there's a forwarder generated in C.
+// otherwise the trait methods are just the inherited default methods from M1.
+trait M2[F] { self: M1[F] =>
+  override def category: Category[X] = null
+  override def category1: Category[Tuple2] = null
+}
+
+abstract class C extends M1[Float] with M2[Float]
+
 object Test extends App {
-  val ms = classOf[C].getMethods.filter(_.getName.startsWith("category"))
-  println(ms.map(_.toGenericString).sorted.mkString("\n"))
+  def t(c: Class[_]) = {
+    val ms = c.getMethods.filter(_.getName.startsWith("category"))
+    println(ms.map(_.toGenericString).sorted.mkString("\n"))
+  }
+  t(classOf[C])
+  t(classOf[M1[_]])
+  t(classOf[M2[_]])
 }
diff --git a/test/junit/scala/issues/BytecodeTest.scala b/test/junit/scala/issues/BytecodeTest.scala
index 18f8b44391..08b984a622 100644
--- a/test/junit/scala/issues/BytecodeTest.scala
+++ b/test/junit/scala/issues/BytecodeTest.scala
@@ -3,11 +3,14 @@ package scala.issues
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 import org.junit.Test
+
 import scala.tools.asm.Opcodes._
 import scala.tools.nsc.backend.jvm.AsmUtils
 import scala.tools.nsc.backend.jvm.CodeGenTools._
 import org.junit.Assert._
+
 import scala.collection.JavaConverters._
+import scala.tools.asm.tree.ClassNode
 import scala.tools.partest.ASMConverters._
 import scala.tools.testing.ClearAfterClass
 
@@ -208,4 +211,134 @@ class BytecodeTest extends ClearAfterClass {
       Label(14), Op(ICONST_0),
       Label(17), Op(IRETURN)))
   }
+
+  object forwarderTestUtils {
+    def findMethods(cls: ClassNode, name: String): List[Method] = cls.methods.iterator.asScala.find(_.name == name).map(convertMethod).toList
+
+    import language.implicitConversions
+    implicit def s2c(s: Symbol)(implicit classes: Map[String, ClassNode]): ClassNode = classes(s.name)
+
+    def checkForwarder(c: ClassNode, target: String) = {
+      val List(f) = findMethods(c, "f")
+      assertSameCode(f, List(VarOp(ALOAD, 0), Invoke(INVOKESPECIAL, target, "f", "()I", false), Op(IRETURN)))
+    }
+  }
+
+  @Test
+  def traitMethodForwarders(): Unit = {
+    import forwarderTestUtils._
+    val code =
+      """trait T1 { def f = 1 }
+        |trait T2 extends T1 { override def f = 2 }
+        |trait T3 { self: T1 => override def f = 3 }
+        |
+        |abstract class A1 { def f: Int }
+        |class A2 { def f: Int = 4 }
+        |
+        |trait T4 extends A1 { def f = 5 }
+        |trait T5 extends A2 { override def f = 6 }
+        |
+        |trait T6 { def f: Int }
+        |trait T7 extends T6 { abstract override def f = super.f + 1 }
+        |
+        |trait T8 { override def clone() = super.clone() }
+        |
+        |class A3 extends T1 { override def f = 7 }
+        |
+        |class C1 extends T1
+        |class C2 extends T2
+        |class C3 extends T1 with T2
+        |class C4 extends T2 with T1
+        |class C5 extends T1 with T3
+        |
+        |// traits extending a class that defines f
+        |class C6 extends T4
+        |class C7 extends T5
+        |class C8 extends A1 with T4
+        |class C9 extends A2 with T5
+        |
+        |// T6: abstract f in trait
+        |class C10 extends T6 with T1
+        |class C11 extends T6 with T2
+        |abstract class C12 extends A1 with T6
+        |class C13 extends A2 with T6
+        |class C14 extends T4 with T6
+        |class C15 extends T5 with T6
+        |
+        |// superclass overrides a trait method
+        |class C16 extends A3
+        |class C17 extends A3 with T1
+        |
+        |// abstract override
+        |class C18 extends T6 { def f = 22 }
+        |class C19 extends C18 with T7
+        |
+        |class C20 extends T8
+      """.stripMargin
+
+    implicit val classes = compileClasses(compiler)(code).map(c => (c.name, c)).toMap
+
+    val noForwarder = List('C1, 'C2, 'C3, 'C4, 'C10, 'C11, 'C12, 'C13, 'C16, 'C17)
+    for (c <- noForwarder) assertEquals(findMethods(c, "f"), Nil)
+
+    checkForwarder('C5, "T3")
+    checkForwarder('C6, "T4")
+    checkForwarder('C7, "T5")
+    checkForwarder('C8, "T4")
+    checkForwarder('C9, "T5")
+    checkForwarder('C14, "T4")
+    checkForwarder('C15, "T5")
+    assertSameSummary(getSingleMethod('C18, "f"), List(BIPUSH, IRETURN))
+    checkForwarder('C19, "T7")
+    assertSameCode(getSingleMethod('C19, "T7$$super$f"), List(VarOp(ALOAD, 0), Invoke(INVOKESPECIAL, "C18", "f", "()I", false), Op(IRETURN)))
+    assertInvoke(getSingleMethod('C20, "clone"), "T8", "clone") // mixin forwarder
+  }
+
+  @Test
+  def noTraitMethodForwardersForOverloads(): Unit = {
+    import forwarderTestUtils._
+    val code =
+      """trait T1 { def f(x: Int) = 0 }
+        |trait T2 { def f(x: String) = 1 }
+        |class C extends T1 with T2
+      """.stripMargin
+    val List(c, t1, t2) = compileClasses(compiler)(code)
+    assertEquals(findMethods(c, "f"), Nil)
+  }
+
+  @Test
+  def traitMethodForwardersForJavaDefaultMethods(): Unit = {
+    import forwarderTestUtils._
+    val j1 = ("interface J1 { int f(); }", "J1.java")
+    val j2 = ("interface J2 { default int f() { return 1; } }", "J2.java")
+    val j3 = ("interface J3 extends J1 { default int f() { return 2; } }", "J3.java")
+    val j4 = ("interface J4 extends J2 { default int f() { return 3; } }", "J4.java")
+    val code =
+      """trait T1 extends J2 { override def f = 4 }
+        |trait T2 { self: J2 => override def f = 5 }
+        |
+        |class K1 extends J2
+        |class K2 extends J1 with J2
+        |class K3 extends J2 with J1
+        |
+        |class K4 extends J3
+        |class K5 extends J3 with J1
+        |class K6 extends J1 with J3
+        |
+        |class K7 extends J4
+        |class K8 extends J4 with J2
+        |class K9 extends J2 with J4
+        |
+        |class K10 extends T1 with J2
+        |class K11 extends J2 with T1
+        |
+        |class K12 extends J2 with T2
+      """.stripMargin
+    implicit val classes = compileClasses(compiler)(code, List(j1, j2, j3, j4)).map(c => (c.name, c)).toMap
+
+    val noForwarder = List('K1, 'K2, 'K3, 'K4, 'K5, 'K6, 'K7, 'K8, 'K9, 'K10, 'K11)
+    for (c <- noForwarder) assertEquals(findMethods(c, "f"), Nil)
+
+    checkForwarder('K12, "T2")
+  }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
index 9079ca248a..15665fca33 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
@@ -458,19 +458,6 @@ class InlinerTest extends ClearAfterClass {
   }
 
   @Test
-  def inlineMixinMethods(): Unit = {
-    val code =
-      """trait T {
-        |  @inline final def f = 1
-        |}
-        |class C extends T
-      """.stripMargin
-    val List(c, t) = compile(code)
-    // the static implementation method is inlined into the mixin, so there's no invocation in the mixin
-    assertNoInvoke(getSingleMethod(c, "f"))
-  }
-
-  @Test
   def inlineTraitInherited(): Unit = {
     val code =
       """trait T {
@@ -545,7 +532,7 @@ class InlinerTest extends ClearAfterClass {
     val List(c, oMirror, oModule, t) = compile(code, allowMessage = i => {count += 1; i.msg contains warn})
     assert(count == 1, count)
 
-    assertNoInvoke(getSingleMethod(oModule, "f"))
+    assertNoInvoke(getSingleMethod(t, "f"))
 
     assertNoInvoke(getSingleMethod(c, "t1"))
     assertNoInvoke(getSingleMethod(c, "t2"))
@@ -1497,10 +1484,12 @@ class InlinerTest extends ClearAfterClass {
   @Test
   def sd86(): Unit = {
     val code =
-      """trait T { @inline def f = 1 } // note that f is not final
-        |class C extends T
+      """trait T1 { @inline def f = 999 }
+        |trait T2 { self: T1 => @inline override def f = 1 } // note that f is not final
+        |class C extends T1 with T2
       """.stripMargin
-    val List(c, t) = compile(code, allowMessage = _ => true)
+    val List(c, t1, t2) = compile(code, allowMessage = _ => true)
+    // the forwarder C.f is inlined, so there's no invocation
     assertSameSummary(getSingleMethod(c, "f"), List(ICONST_1, IRETURN))
   }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala
index 4db2657c1b..46b3ad3f8e 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala
@@ -113,10 +113,6 @@ class ScalaInlineInfoTest extends ClearAfterClass {
     val infoC = inlineInfo(c)
     val expectC = InlineInfo(false, None, Map(
       "O()LT$O$;"                             -> MethodInlineInfo(true ,false,false),
-      "f1()I"                                 -> MethodInlineInfo(false,false,false),
-      "f3()I"                                 -> MethodInlineInfo(false,false,false),
-      "f4()Ljava/lang/String;"                -> MethodInlineInfo(false,true ,false),
-      "f5()I"                                 -> MethodInlineInfo(true ,false,false),
       "f6()I"                                 -> MethodInlineInfo(false,false,false),
       "x1()I"                                 -> MethodInlineInfo(false,false,false),
       "T$_setter_$x1_$eq(I)V"                 -> MethodInlineInfo(false,false,false),