From 40f7fce0af1da614d99048b024e1ff579635f0f2 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 2 Aug 2016 11:43:36 -0700 Subject: 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. --- .../tools/nsc/backend/jvm/BCodeBodyBuilder.scala | 8 ++++-- .../scala/tools/nsc/backend/jvm/BCodeHelpers.scala | 2 +- .../tools/nsc/backend/jvm/BCodeSkelBuilder.scala | 31 +++++++++++++--------- .../tools/nsc/backend/jvm/BTypesFromSymbols.scala | 2 +- .../scala/reflect/internal/StdAttachments.scala | 2 ++ .../scala/reflect/runtime/JavaUniverseForce.scala | 1 + .../scala/collection/mutable/OpenHashMapTest.scala | 2 +- .../scala/tools/nsc/backend/jvm/BytecodeTest.scala | 6 ++--- .../jvm/opt/InlinerSeparateCompilationTest.scala | 4 +-- .../tools/nsc/backend/jvm/opt/InlinerTest.scala | 14 +++++----- 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)) } -- cgit v1.2.3