From a0590aa9ba45e83c8c8d496b8ab132966b1a7a95 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Fri, 15 Jul 2016 12:17:52 +0200 Subject: SD-182 compiler option -Xgen-mixin-forwarders Introduce a compiler option -Xgen-mixin-forwarders to always generate mixin forwarder methods. --- .../scala/tools/nsc/settings/ScalaSettings.scala | 1 + src/compiler/scala/tools/nsc/transform/Mixin.scala | 78 ++++++++++++---------- test/junit/scala/lang/traits/BytecodeTest.scala | 29 ++++++++ 3 files changed, 73 insertions(+), 35 deletions(-) diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala index 4d236b226d..dae8539c66 100644 --- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala @@ -133,6 +133,7 @@ trait ScalaSettings extends AbsScalaSettings val XnoPatmatAnalysis = BooleanSetting ("-Xno-patmat-analysis", "Don't perform exhaustivity/unreachability analysis. Also, ignore @switch annotation.") val XfullLubs = BooleanSetting ("-Xfull-lubs", "Retains pre 2.10 behavior of less aggressive truncation of least upper bounds.") + val XgenMixinForwarders = BooleanSetting("-Xgen-mixin-forwarders", "Generate forwarder methods in classes inhering concrete methods from traits.") // XML parsing options object XxmlSettings extends MultiChoiceEnumeration { diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala index d62b77dac2..b787f64846 100644 --- a/src/compiler/scala/tools/nsc/transform/Mixin.scala +++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala @@ -246,43 +246,51 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL { case NoSymbol => val isMemberOfClazz = clazz.info.findMember(member.name, 0, 0L, stableOnly = false).alternatives.contains(member) if (isMemberOfClazz) { - // `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 of `clazz` defines a 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 a parent interface `p` of - // `clazz`, and the `mixinClass` does not itself extend `p`. 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. - - @tailrec - def existsCompetingMethod(baseClasses: List[Symbol]): Boolean = baseClasses match { - case baseClass :: rest => - if (baseClass ne mixinClass) { - val m = member.overriddenSymbol(baseClass) - val isCompeting = m.exists && { - !m.owner.isTraitOrInterface || - (!m.isDeferred && !mixinClass.isNonBottomSubClass(m.owner)) - } - isCompeting || existsCompetingMethod(rest) - } else existsCompetingMethod(rest) - - case _ => false + def genForwarder(): Unit = { + cloneAndAddMixinMember(mixinClass, member).asInstanceOf[TermSymbol] setAlias member } - if (existsCompetingMethod(clazz.baseClasses)) - cloneAndAddMixinMember(mixinClass, member).asInstanceOf[TermSymbol] setAlias member - else if (!settings.nowarnDefaultJunitMethods && JUnitTestClass.exists && member.hasAnnotation(JUnitTestClass)) - warning(member.pos, "JUnit tests in traits that are compiled as default methods are not executed by JUnit 4. JUnit 5 will fix this issue.") + if (settings.XgenMixinForwarders) genForwarder() + else { + + // `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 of `clazz` defines a 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 a parent interface `p` of + // `clazz`, and the `mixinClass` does not itself extend `p`. 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. + + @tailrec + def existsCompetingMethod(baseClasses: List[Symbol]): Boolean = baseClasses match { + case baseClass :: rest => + if (baseClass ne mixinClass) { + val m = member.overriddenSymbol(baseClass) + val isCompeting = m.exists && { + !m.owner.isTraitOrInterface || + (!m.isDeferred && !mixinClass.isNonBottomSubClass(m.owner)) + } + isCompeting || existsCompetingMethod(rest) + } else existsCompetingMethod(rest) + + case _ => false + } + + if (existsCompetingMethod(clazz.baseClasses)) + genForwarder() + else if (!settings.nowarnDefaultJunitMethods && JUnitTestClass.exists && member.hasAnnotation(JUnitTestClass)) + warning(member.pos, "JUnit tests in traits that are compiled as default methods are not executed by JUnit 4. JUnit 5 will fix this issue.") + } } case _ => diff --git a/test/junit/scala/lang/traits/BytecodeTest.scala b/test/junit/scala/lang/traits/BytecodeTest.scala index ec8508df99..e6c74b86ab 100644 --- a/test/junit/scala/lang/traits/BytecodeTest.scala +++ b/test/junit/scala/lang/traits/BytecodeTest.scala @@ -230,6 +230,35 @@ class BytecodeTest extends BytecodeTesting { List(ALOAD, ILOAD, PUTFIELD, ALOAD, ACONST_NULL, "", RETURN)) } + @Test + def mixinForwarders(): Unit = { + val code = + """trait T { def f = 1 } + |class C extends T + """.stripMargin + val List(c1, _) = compileClasses(code) + val List(c2, _) = newCompiler(extraArgs = "-Xgen-mixin-forwarders").compileClasses(code) + assert(getMethods(c1, "f").isEmpty) + assertSameCode(getMethod(c2, "f"), + List(VarOp(ALOAD, 0), Invoke(INVOKESTATIC, "T", "f$", "(LT;)I", true), Op(IRETURN))) + } + + @Test + def sd143(): Unit = { + // this tests the status quo, which is wrong. + val code = + """class A { def m = 1 } + |class B extends A { override def m = 2 } + |trait T extends A + |class C extends B with T { + | override def m = super[T].m // should invoke A.m + |} + """.stripMargin + val List(_, _, c, _) = compileClasses(code) + // even though the bytecode refers to A.m, invokespecial will resolve to B.m + assertSameCode(getMethod(c, "m"), + List(VarOp(ALOAD, 0), Invoke(INVOKESPECIAL, "A", "m", "()I", false), Op(IRETURN))) + } } object invocationReceiversTestCode { -- cgit v1.2.3