SD-192 Change scheme for trait super accessors

Rather than putting the code of a trait method body into a static method,
leave it in the default method. The static method (needed as the target
of the super calls) now uses `invokespecial` to exactly call that method.

10 files changed, 42 insertions, 30 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
index 6f9682f434..bac84a4959 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
@@ -657,9 +657,13 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
           } else if (isPrimitive(sym)) { // primitive method call
             generatedType = genPrimitiveOp(app, expectedType)
           } else { // normal method call
+            def isTraitSuperAccessorBodyCall = app.hasAttachment[UseInvokeSpecial.type]
             val invokeStyle =
-              if (sym.isStaticMember) InvokeStyle.Static
+              if (sym.isStaticMember)
+                InvokeStyle.Static
               else if (sym.isPrivate || sym.isClassConstructor) InvokeStyle.Special
+              else if (isTraitSuperAccessorBodyCall)
+                InvokeStyle.Special
               else InvokeStyle.Virtual
 
             if (invokeStyle.hasInstance) genLoadQualifier(fun)
@@ -1077,7 +1081,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
         assert(receiverClass == methodOwner, s"for super call, expecting $receiverClass == $methodOwner")
         if (receiverClass.isTrait && !receiverClass.isJavaDefined) {
           val staticDesc = MethodBType(typeToBType(method.owner.info) :: bmType.argumentTypes, bmType.returnType).descriptor
-          val staticName = traitImplMethodName(method).toString
+          val staticName = traitSuperAccessorName(method).toString
           bc.invokestatic(receiverName, staticName, staticDesc, isInterface, pos)
         } else {
           if (receiverClass.isTraitOrInterface) {
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
index e1decaba3e..18e7500172 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
@@ -52,7 +52,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
 
   def needsStaticImplMethod(sym: Symbol) = sym.hasAttachment[global.mixer.NeedStaticImpl.type]
 
-  final def traitImplMethodName(sym: Symbol): Name = {
+  final def traitSuperAccessorName(sym: Symbol): Name = {
     val name = sym.javaSimpleName
     if (sym.isMixinConstructor) name
     else name.append(nme.NAME_JOIN_STRING)
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
index d4d532f4df..481f71cb5d 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
@@ -8,12 +8,12 @@ package scala.tools.nsc
 package backend
 package jvm
 
-import scala.collection.{ mutable, immutable }
+import scala.collection.{immutable, mutable}
 import scala.tools.nsc.symtab._
-
 import scala.tools.asm
 import GenBCode._
 import BackendReporting._
+import scala.tools.nsc.backend.jvm.BCodeHelpers.InvokeStyle
 
 /*
  *
@@ -483,18 +483,23 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
         case dd : DefDef =>
           val sym = dd.symbol
           if (needsStaticImplMethod(sym)) {
-            val staticDefDef = global.gen.mkStatic(dd, traitImplMethodName(sym), _.cloneSymbol)
-            val forwarderDefDef = {
-              val forwarderBody = Apply(global.gen.mkAttributedRef(staticDefDef.symbol), This(sym.owner).setType(sym.owner.typeConstructor) :: dd.vparamss.head.map(p => global.gen.mkAttributedIdent(p.symbol))).setType(sym.info.resultType)
-              // we don't want to the optimizer to inline the static method into the forwarder. Instead,
-              // the backend has a special case to transitively inline into a callsite of the forwarder
-              // when the forwarder itself is inlined.
-              forwarderBody.updateAttachment(NoInlineCallsiteAttachment)
-              deriveDefDef(dd)(_ => global.atPos(dd.pos)(forwarderBody))
-            }
-            genDefDef(staticDefDef)
-            if (!sym.isMixinConstructor)
+            if (sym.isMixinConstructor) {
+              val statified = global.gen.mkStatic(dd, sym.name, _.cloneSymbol)
+              genDefDef(statified)
+            } else {
+              val forwarderDefDef = {
+                val dd1 = global.gen.mkStatic(deriveDefDef(dd)(_ => EmptyTree), traitSuperAccessorName(sym), _.cloneSymbol)
+                dd1.symbol.setFlag(Flags.ARTIFACT).resetFlag(Flags.OVERRIDE)
+                val selfParam :: realParams = dd1.vparamss.head.map(_.symbol)
+                deriveDefDef(dd1)(_ =>
+                  atPos(dd1.pos)(
+                    Apply(Select(global.gen.mkAttributedIdent(selfParam).setType(sym.owner.typeConstructor), dd.symbol),
+                    realParams.map(global.gen.mkAttributedIdent)).updateAttachment(UseInvokeSpecial))
+                )
+              }
               genDefDef(forwarderDefDef)
+              genDefDef(dd)
+            }
           } else genDefDef(dd)
 
         case Template(_, _, body) => body foreach gen
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
index 477afaa91b..09e82de89b 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
@@ -596,7 +596,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
             annotatedNoInline = methodSym.hasAnnotation(ScalaNoInlineClass))
 
           if (needsStaticImplMethod(methodSym)) {
-            val staticName = traitImplMethodName(methodSym).toString
+            val staticName = traitSuperAccessorName(methodSym).toString
             val selfParam = methodSym.newSyntheticValueParam(methodSym.owner.typeConstructor, nme.SELF)
             val staticMethodType = methodSym.info match {
               case mt @ MethodType(params, res) => copyMethodType(mt, selfParam :: params, res)
diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala
index 76e34153c9..78f360409d 100644
--- a/src/reflect/scala/reflect/internal/StdAttachments.scala
+++ b/src/reflect/scala/reflect/internal/StdAttachments.scala
@@ -76,4 +76,6 @@ trait StdAttachments {
     * in place of the outer parameter, can help callers to avoid capturing the outer instance.
     */
   case object OuterArgCanBeElided extends PlainAttachment
+
+  case object UseInvokeSpecial extends PlainAttachment
 }
diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
index caef5535b4..f55b33959a 100644
--- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
+++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
@@ -46,6 +46,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.NoInlineCallsiteAttachment
     this.InlineCallsiteAttachment
     this.OuterArgCanBeElided
+    this.UseInvokeSpecial
     this.noPrint
     this.typeDebug
     this.Range
diff --git a/test/junit/scala/collection/mutable/OpenHashMapTest.scala b/test/junit/scala/collection/mutable/OpenHashMapTest.scala
index b6cddf2101..90f6be6ee5 100644
--- a/test/junit/scala/collection/mutable/OpenHashMapTest.scala
+++ b/test/junit/scala/collection/mutable/OpenHashMapTest.scala
@@ -1,6 +1,6 @@
 package scala.collection.mutable
 
-import org.junit.Test
+import org.junit.{Ignore, Test}
 import org.junit.Assert._
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
diff --git a/test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala b/test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala
index 5904cb2441..b09a41969e 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala
@@ -169,7 +169,7 @@ class BytecodeTest extends BytecodeTesting {
     assertEquals(x.end, labels(7))
   }
 
-  @Test // wrong line numbers for rewritten `this` references in trait static methods
+  @Test
   def sd186_traitLineNumber(): Unit = {
     val code =
       """trait T {
@@ -182,9 +182,9 @@ class BytecodeTest extends BytecodeTesting {
     val t = compileClass(code)
     val tMethod = getMethod(t, "t$")
     val invoke = Invoke(INVOKEVIRTUAL, "java/lang/Object", "toString", "()Ljava/lang/String;", false)
+    // ths static accessor is positioned at the line number of the accessed method.
     assertSameCode(tMethod.instructions,
-      List(Label(0), LineNumber(3, Label(0)), VarOp(ALOAD, 0), invoke, Op(POP),
-        Label(5), LineNumber(4, Label(5)), VarOp(ALOAD, 0), invoke, Op(POP), Op(RETURN), Label(11))
+      List(Label(0), LineNumber(2, Label(0)), VarOp(ALOAD, 0), Invoke(INVOKESPECIAL, "T", "t", "()V", true), Op(RETURN), Label(4))
     )
   }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerSeparateCompilationTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerSeparateCompilationTest.scala
index 85df42e069..a2513cacdc 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerSeparateCompilationTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerSeparateCompilationTest.scala
@@ -97,7 +97,7 @@ class InlinerSeparateCompilationTest {
       """.stripMargin
 
     val List(a, t) = compileClassesSeparately(List(codeA, assembly), args)
-    assertNoInvoke(getMethod(t, "f$"))
-    assertNoInvoke(getMethod(a, "n$"))
+    assertNoInvoke(getMethod(t, "f"))
+    assertNoInvoke(getMethod(a, "n"))
   }
 }
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 539c66a0d5..29a23df784 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
@@ -518,7 +518,7 @@ class InlinerTest extends BytecodeTesting {
     val List(c, oMirror, oModule, t) = compile(code, allowMessage = i => {count += 1; i.msg contains warn})
     assert(count == 1, count)
 
-    assertNoInvoke(getMethod(t, "f$"))
+    assertNoInvoke(getMethod(t, "f"))
 
     assertNoInvoke(getMethod(c, "t1"))
     assertNoInvoke(getMethod(c, "t2"))
@@ -544,9 +544,9 @@ class InlinerTest extends BytecodeTesting {
 
     val List(assembly, c, t) = compile(code)
 
-    assertNoInvoke(getMethod(t, "f$"))
+    assertNoInvoke(getMethod(t, "f"))
 
-    assertNoInvoke(getMethod(assembly, "n$"))
+    assertNoInvoke(getMethod(assembly, "n"))
 
     assertNoInvoke(getMethod(c, "t1"))
     assertNoInvoke(getMethod(c, "t2"))
@@ -622,8 +622,8 @@ class InlinerTest extends BytecodeTesting {
     val List(ca, cb, t1, t2a, t2b) = compile(code, allowMessage = i => {count += 1; i.msg contains warning})
     assert(count == 4, count) // see comments, f is not inlined 4 times
 
-    assertNoInvoke(getMethod(t2a, "g2a$"))
-    assertInvoke(getMethod(t2b, "g2b$"), "T1", "f")
+    assertNoInvoke(getMethod(t2a, "g2a"))
+    assertInvoke(getMethod(t2b, "g2b"), "T1", "f")
 
     assertInvoke(getMethod(ca, "m1a"), "T1", "f")
     assertNoInvoke(getMethod(ca, "m2a"))            // no invoke, see comment on def g2a
@@ -682,8 +682,8 @@ class InlinerTest extends BytecodeTesting {
         |}
       """.stripMargin
     val List(c, t) = compile(code)
-    val t1 = getMethod(t, "t1$")
-    val t2 = getMethod(t, "t2$")
+    val t1 = getMethod(t, "t1")
+    val t2 = getMethod(t, "t2")
     val cast = TypeOp(CHECKCAST, "C")
     Set(t1, t2).foreach(m => assert(m.instructions.contains(cast), m.instructions))
   }