From dec6cc670f1a5c8d585afaf6079fc96ade14f306 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 26 Nov 2020 13:02:39 +0100 Subject: [PATCH] Fix #8599: Emit trait init methods only as static methods. Concrete trait methods are encoded as two methods in the back-end: * a default method containing the real body, and * a static forwarder, to use for super calls (due to JVM reasons). Previously, we did that also for the trait initializer methods `$init$`. However, that causes unrelated default methods to be inherited by Scala classes that extend several traits. In turn, that prevents Java classes from extending such classes. Now, for the `$init$` methods, instead of creating a static forwarder, we *move* the entire body to a static method. Therefore, we only create a static method, and no default method. This corresponds to what scalac does as well (both what we do and how we do it), although the previous "discrepancy" was not causing any incompatibility between Scala 2 and 3 per se. --- .../tools/backend/jvm/BCodeSkelBuilder.scala | 73 ++++++++++++++++--- tests/run/i8599/JavaClass_2.java | 5 ++ tests/run/i8599/ScalaDefs_1.scala | 9 +++ tests/run/i8599/Test_3.scala | 7 ++ tests/run/junitForwarders/C_1.scala | 2 +- tests/run/traitNoInit.scala | 16 +--- 6 files changed, 87 insertions(+), 25 deletions(-) create mode 100644 tests/run/i8599/JavaClass_2.java create mode 100644 tests/run/i8599/ScalaDefs_1.scala create mode 100644 tests/run/i8599/Test_3.scala diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index a0f7d96e5535..e3f740e3abee 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -17,6 +17,7 @@ import dotty.tools.dotc.core.Decorators._ import dotty.tools.dotc.core.Flags._ import dotty.tools.dotc.core.StdNames._ import dotty.tools.dotc.core.NameKinds._ +import dotty.tools.dotc.core.Names.TermName import dotty.tools.dotc.core.Symbols._ import dotty.tools.dotc.core.Types._ import dotty.tools.dotc.core.Contexts._ @@ -577,6 +578,13 @@ trait BCodeSkelBuilder extends BCodeHelpers { * trait method. This is required for super calls to this method, which * go through the static forwarder in order to work around limitations * of the JVM. + * + * For the $init$ method, we must not leave it as a default method, but + * instead we must put the whole body in the static method. If we leave + * it as a default method, Java classes cannot extend Scala classes that + * extend several Scala traits, since they then inherit unrelated default + * $init$ methods. See #8599. scalac does the same thing. + * * In theory, this would go in a separate MiniPhase, but it would have to * sit in a MegaPhase of its own between GenSJSIR and GenBCode, so the cost * is not worth it. We directly do it in this back-end instead, which also @@ -586,9 +594,13 @@ trait BCodeSkelBuilder extends BCodeHelpers { val needsStaticImplMethod = claszSymbol.isInterface && !dd.rhs.isEmpty && !sym.isPrivate && !sym.isStaticMember if needsStaticImplMethod then - genStaticForwarderForDefDef(dd) - - genDefDef(dd) + if sym.name == nme.TRAIT_CONSTRUCTOR then + genTraitConstructorDefDef(dd) + else + genStaticForwarderForDefDef(dd) + genDefDef(dd) + else + genDefDef(dd) case tree: Template => val body = @@ -629,6 +641,42 @@ trait BCodeSkelBuilder extends BCodeHelpers { } // end of method initJMethod + private def genTraitConstructorDefDef(dd: DefDef): Unit = + val statifiedDef = makeStatifiedDefDef(dd) + genDefDef(statifiedDef) + + /** Creates a copy of the given DefDef that is static and where an explicit + * self parameter represents the original `this` value. + * + * Example: from + * {{{ + * trait Enclosing { + * def foo(x: Int): String = this.toString() + x + * } + * }}} + * the statified version of `foo` would be + * {{{ + * static def foo($self: Enclosing, x: Int): String = $self.toString() + x + * }}} + */ + private def makeStatifiedDefDef(dd: DefDef): DefDef = + val origSym = dd.symbol.asTerm + val newSym = makeStatifiedDefSymbol(origSym, origSym.name) + tpd.DefDef(newSym, { paramRefss => + val selfParamRef :: regularParamRefs = paramRefss.head + val enclosingClass = origSym.owner.asClass + new TreeTypeMap( + typeMap = _.substThis(enclosingClass, selfParamRef.symbol.termRef) + .subst(dd.vparamss.head.map(_.symbol), regularParamRefs.map(_.symbol.termRef)), + treeMap = { + case tree: This if tree.symbol == enclosingClass => selfParamRef + case tree => tree + }, + oldOwners = origSym :: Nil, + newOwners = newSym :: Nil + ).transform(dd.rhs) + }) + private def genStaticForwarderForDefDef(dd: DefDef): Unit = val forwarderDef = makeStaticForwarder(dd) genDefDef(forwarderDef) @@ -646,20 +694,23 @@ trait BCodeSkelBuilder extends BCodeHelpers { */ private def makeStaticForwarder(dd: DefDef): DefDef = val origSym = dd.symbol.asTerm - val name = traitSuperAccessorName(origSym) + val name = traitSuperAccessorName(origSym).toTermName + val sym = makeStatifiedDefSymbol(origSym, name) + tpd.DefDef(sym, { paramss => + val params = paramss.head + tpd.Apply(params.head.select(origSym), params.tail) + .withAttachment(BCodeHelpers.UseInvokeSpecial, ()) + }) + + private def makeStatifiedDefSymbol(origSym: TermSymbol, name: TermName): TermSymbol = val info = origSym.info match case mt: MethodType => MethodType(nme.SELF :: mt.paramNames, origSym.owner.typeRef :: mt.paramInfos, mt.resType) - val sym = origSym.copy( + origSym.copy( name = name.toTermName, flags = Method | JavaStatic, info = info - ) - tpd.DefDef(sym.asTerm, { paramss => - val params = paramss.head - tpd.Apply(params.head.select(origSym), params.tail) - .withAttachment(BCodeHelpers.UseInvokeSpecial, ()) - }) + ).asTerm def genDefDef(dd: DefDef): Unit = { val rhs = dd.rhs diff --git a/tests/run/i8599/JavaClass_2.java b/tests/run/i8599/JavaClass_2.java new file mode 100644 index 000000000000..86ee08044f07 --- /dev/null +++ b/tests/run/i8599/JavaClass_2.java @@ -0,0 +1,5 @@ +public class JavaClass_2 extends ScalaClass { + public int c() { + return a() + b(); + } +} diff --git a/tests/run/i8599/ScalaDefs_1.scala b/tests/run/i8599/ScalaDefs_1.scala new file mode 100644 index 000000000000..df4e5f23d86b --- /dev/null +++ b/tests/run/i8599/ScalaDefs_1.scala @@ -0,0 +1,9 @@ +trait A { + val a: Int = 1 +} + +trait B { + val b: Int = 2 +} + +class ScalaClass extends A with B diff --git a/tests/run/i8599/Test_3.scala b/tests/run/i8599/Test_3.scala new file mode 100644 index 000000000000..144d2126667d --- /dev/null +++ b/tests/run/i8599/Test_3.scala @@ -0,0 +1,7 @@ +object Test { + def main(args: Array[String]): Unit = + val obj = new JavaClass_2 + assert(obj.a == 1) + assert(obj.b == 2) + assert(obj.c() == 3) +} diff --git a/tests/run/junitForwarders/C_1.scala b/tests/run/junitForwarders/C_1.scala index 04bd263bf791..cc5f94eed97f 100644 --- a/tests/run/junitForwarders/C_1.scala +++ b/tests/run/junitForwarders/C_1.scala @@ -18,6 +18,6 @@ object Test extends App { } check(classOf[C], "foo - @org.junit.Test()") // scala/scala-dev#213, scala/scala#5570: `foo$` should not have the @Test annotation - check(classOf[T], "$init$ - ;$init$ - ;foo - @org.junit.Test();foo$ - ") + check(classOf[T], "$init$ - ;foo - @org.junit.Test();foo$ - ") check(classOf[U], "bar - @org.junit.Test();bar$ - ") } diff --git a/tests/run/traitNoInit.scala b/tests/run/traitNoInit.scala index a5f75dc02610..fcfd8a6d535e 100644 --- a/tests/run/traitNoInit.scala +++ b/tests/run/traitNoInit.scala @@ -9,21 +9,11 @@ trait WithInit { trait Bar(x: Int) -class NoInitClass extends NoInit() with Bar(1) { - def meth(x: Int) = x -} - -class WithInitClass extends WithInit() with Bar(1) { - def meth(x: Int) = x -} - object Test { def hasInit(cls: Class[_]) = cls.getMethods.map(_.toString).exists(_.contains("$init$")) def main(args: Array[String]): Unit = { - val noInit = new NoInitClass {} - val withInit = new WithInitClass {} - - assert(!hasInit(noInit.getClass)) - assert(hasInit(withInit.getClass)) + assert(!hasInit(classOf[NoInit])) + assert(hasInit(classOf[WithInit])) + assert(!hasInit(classOf[Bar])) } }