Skip to content

Fix #8599: Emit trait init methods only as static methods. #10509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 62 additions & 11 deletions compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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
Expand All @@ -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 =
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions tests/run/i8599/JavaClass_2.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
public class JavaClass_2 extends ScalaClass {
public int c() {
return a() + b();
}
}
9 changes: 9 additions & 0 deletions tests/run/i8599/ScalaDefs_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
trait A {
val a: Int = 1
}

trait B {
val b: Int = 2
}

class ScalaClass extends A with B
7 changes: 7 additions & 0 deletions tests/run/i8599/Test_3.scala
Original file line number Diff line number Diff line change
@@ -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)
}
2 changes: 1 addition & 1 deletion tests/run/junitForwarders/C_1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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$ - ")
}
16 changes: 3 additions & 13 deletions tests/run/traitNoInit.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
}
}