From 6d0f9ca3ae4c7130c1ee18e0e21f283ff6cb21f3 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 21 Mar 2019 15:37:16 +0100 Subject: [PATCH] Emit mixin forwarders as bridges This is the same scheme I proposed in https://github.com/scala/scala/pull/7843 which sidesteps all the issues regarding mixin forwarders and generic signatures, see the discussion in that PR for more information. Tests imported from scalac, some of the comments in them might not be correct for Dotty anymore. --- .../dotty/tools/dotc/transform/Mixin.scala | 2 +- .../dotty/tools/dotc/transform/MixinOps.scala | 4 +- .../test/dotc/run-test-pickling.blacklist | 2 + tests/run/mixin-bridge-methods.scala | 14 --- tests/run/mixin-forwarder-overload/A.scala | 9 ++ tests/run/mixin-forwarder-overload/Test.java | 7 ++ tests/run/mixin-signatures.check | 77 +++++++++++++ tests/run/mixin-signatures.scala | 105 ++++++++++++++++++ tests/run/t3452b-bcode/J_2.java | 6 - tests/run/t3452b-bcode/S_1.scala | 17 --- tests/run/t3452b-bcode/S_3.scala | 5 - tests/run/t3452d/A.scala | 2 +- tests/run/t3452d/Test.java | 8 +- tests/run/t3452g/A.scala | 4 +- tests/run/t3452g/Test.java | 16 +-- tests/run/t3452h.scala | 19 +--- tests/run/t7932.check | 6 + tests/run/t7932.scala | 30 +++++ tests/run/t8905/DoubleRDD.scala | 9 ++ tests/run/t8905/Test.java | 20 ++++ 20 files changed, 284 insertions(+), 78 deletions(-) delete mode 100644 tests/run/mixin-bridge-methods.scala create mode 100644 tests/run/mixin-forwarder-overload/A.scala create mode 100644 tests/run/mixin-forwarder-overload/Test.java create mode 100644 tests/run/mixin-signatures.check create mode 100644 tests/run/mixin-signatures.scala delete mode 100644 tests/run/t3452b-bcode/J_2.java delete mode 100644 tests/run/t3452b-bcode/S_1.scala delete mode 100644 tests/run/t3452b-bcode/S_3.scala create mode 100644 tests/run/t7932.check create mode 100644 tests/run/t7932.scala create mode 100644 tests/run/t8905/DoubleRDD.scala create mode 100644 tests/run/t8905/Test.java diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 8a44af8f21f0..125c64f78f15 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -262,7 +262,7 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => for (meth <- mixin.info.decls.toList if needsMixinForwarder(meth)) yield { util.Stats.record("mixin forwarders") - transformFollowing(polyDefDef(mkForwarderSym(meth.asTerm), forwarderRhsFn(meth))) + transformFollowing(polyDefDef(mkForwarderSym(meth.asTerm, Bridge), forwarderRhsFn(meth))) } diff --git a/compiler/src/dotty/tools/dotc/transform/MixinOps.scala b/compiler/src/dotty/tools/dotc/transform/MixinOps.scala index 806d0676c700..3ac082959a46 100644 --- a/compiler/src/dotty/tools/dotc/transform/MixinOps.scala +++ b/compiler/src/dotty/tools/dotc/transform/MixinOps.scala @@ -18,11 +18,11 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(implicit ctx: Cont map(n => ctx.getClassIfDefined("org.junit." + n)). filter(_.exists) - def mkForwarderSym(member: TermSymbol): TermSymbol = { + def mkForwarderSym(member: TermSymbol, extraFlags: FlagSet = EmptyFlags): TermSymbol = { val res = member.copy( owner = cls, name = member.name.stripScala2LocalSuffix, - flags = member.flags &~ Deferred | Synthetic | Artifact, + flags = member.flags &~ Deferred | Synthetic | Artifact | extraFlags, info = cls.thisType.memberInfo(member)).enteredAfter(thisPhase).asTerm res.addAnnotations(member.annotations.filter(_.symbol != defn.TailrecAnnot)) res diff --git a/compiler/test/dotc/run-test-pickling.blacklist b/compiler/test/dotc/run-test-pickling.blacklist index 73d8435053cb..27c555e3f628 100644 --- a/compiler/test/dotc/run-test-pickling.blacklist +++ b/compiler/test/dotc/run-test-pickling.blacklist @@ -13,3 +13,5 @@ derive-generic.scala deriving-interesting-prefixes.scala instances.scala instances-anonymous.scala +mixin-forwarder-overload +t8905 diff --git a/tests/run/mixin-bridge-methods.scala b/tests/run/mixin-bridge-methods.scala deleted file mode 100644 index e0340ebb125a..000000000000 --- a/tests/run/mixin-bridge-methods.scala +++ /dev/null @@ -1,14 +0,0 @@ -trait Foo { - def getFoo() = "foo" -} - -class Sub extends Foo { - def getBar() = "bar" -} - -object Test { - def main(args: Array[String]): Unit = { - val ms = classOf[Sub].getDeclaredMethods - assert(ms forall (x => !x.isBridge), ms mkString " ") - } -} diff --git a/tests/run/mixin-forwarder-overload/A.scala b/tests/run/mixin-forwarder-overload/A.scala new file mode 100644 index 000000000000..9eb20ac21414 --- /dev/null +++ b/tests/run/mixin-forwarder-overload/A.scala @@ -0,0 +1,9 @@ +case class Foo(x: Int) + +trait A[X] { + def concat[Dummy](suffix: Int): Dummy = ??? +} + +class Bar extends A[Foo] { + def concat(suffix: Int): Foo = Foo(0) +} diff --git a/tests/run/mixin-forwarder-overload/Test.java b/tests/run/mixin-forwarder-overload/Test.java new file mode 100644 index 000000000000..e5cecc71c64d --- /dev/null +++ b/tests/run/mixin-forwarder-overload/Test.java @@ -0,0 +1,7 @@ +public class Test { + public static void main(String[] args) { + Bar bar = new Bar(); + Foo x = bar.concat(0); + System.out.println(x); + } +} diff --git a/tests/run/mixin-signatures.check b/tests/run/mixin-signatures.check new file mode 100644 index 000000000000..98979ab8d99b --- /dev/null +++ b/tests/run/mixin-signatures.check @@ -0,0 +1,77 @@ +class Test$bar1$ { + public java.lang.Object Test$bar1$.f(java.lang.Object) + public java.lang.String Test$bar1$.f(java.lang.Object) + public java.lang.String Test$bar1$.g(java.lang.String) + public java.lang.Object Test$bar1$.g(java.lang.Object) + public java.lang.String Test$bar1$.g(java.lang.Object) + public java.lang.Object Test$bar1$.h(java.lang.Object) +} + +class Test$bar2$ { + public java.lang.Object Test$bar2$.f(java.lang.Object) + public java.lang.Object Test$bar2$.f(java.lang.String) + public java.lang.String Test$bar2$.g(java.lang.String) + public java.lang.Object Test$bar2$.g(java.lang.Object) + public java.lang.Object Test$bar2$.g(java.lang.String) + public java.lang.Object Test$bar2$.h(java.lang.Object) +} + +class Test$bar3$ { + public java.lang.String Foo3.f(java.lang.Object) + generic: public java.lang.String Foo3.f(T) + public java.lang.Object Foo3.f(java.lang.Object) + public java.lang.String Test$bar3$.g(java.lang.String) + public java.lang.Object Test$bar3$.g(java.lang.Object) + public java.lang.String Test$bar3$.g(java.lang.Object) + public java.lang.Object Foo3.h(java.lang.Object) +} + +class Test$bar4$ { + public java.lang.Object Foo4.f(java.lang.String) + generic: public R Foo4.f(java.lang.String) + public java.lang.Object Foo4.f(java.lang.Object) + public java.lang.String Test$bar4$.g(java.lang.String) + public java.lang.Object Test$bar4$.g(java.lang.Object) + public java.lang.Object Test$bar4$.g(java.lang.String) + public java.lang.Object Foo4.h(java.lang.Object) +} + +class Test$bar5$ { + public java.lang.String Test$bar5$.f(java.lang.String) + public java.lang.Object Test$bar5$.f(java.lang.Object) + public java.lang.Object Test$bar5$.f(java.lang.String) + public java.lang.String Test$bar5$.f(java.lang.Object) + public java.lang.String Test$bar5$.g(java.lang.String) + public java.lang.Object Test$bar5$.g(java.lang.Object) + public java.lang.Object Test$bar5$.g(java.lang.String) + public java.lang.String Test$bar5$.g(java.lang.Object) + public java.lang.Object Test$bar5$.h(java.lang.Object) +} + +interface Foo1 { + public abstract java.lang.Object Base.f(java.lang.Object) + generic: public abstract R Base.f(T) + public default java.lang.String Foo1.f(java.lang.Object) + generic: public default java.lang.String Foo1.f(T) + public abstract java.lang.Object Base.g(java.lang.Object) + generic: public abstract R Base.g(T) + public abstract java.lang.String Foo1.g(java.lang.Object) + generic: public abstract java.lang.String Foo1.g(T) + public default java.lang.Object Base.h(java.lang.Object) + generic: public default R Base.h(T) +} + +interface Foo2 { + public abstract java.lang.Object Base.f(java.lang.Object) + generic: public abstract R Base.f(T) + public default java.lang.Object Foo2.f(java.lang.String) + generic: public default R Foo2.f(java.lang.String) + public abstract java.lang.Object Base.g(java.lang.Object) + generic: public abstract R Base.g(T) + public abstract java.lang.Object Foo2.g(java.lang.String) + generic: public abstract R Foo2.g(java.lang.String) + public default java.lang.Object Base.h(java.lang.Object) + generic: public default R Base.h(T) +} + +000000000000000000000000000000000000 diff --git a/tests/run/mixin-signatures.scala b/tests/run/mixin-signatures.scala new file mode 100644 index 000000000000..48e345e8c645 --- /dev/null +++ b/tests/run/mixin-signatures.scala @@ -0,0 +1,105 @@ +trait Base[T, R] { + def f(x: T): R + def g(x: T): R + def h(x: T): R = null.asInstanceOf[R] +} + +trait Foo1[T] extends Base[T, String] { + def f(x: T): String = null + def g(x: T): String +} +trait Foo2[R] extends Base[String, R] { + def f(x: String): R = { print(x.length) ; null.asInstanceOf[R] } + def g(x: String): R +} +abstract class Foo3[T] extends Base[T, String] { + def f(x: T): String = "" + def g(x: T): String +} +abstract class Foo4[R] extends Base[String, R] { + def f(x: String): R = { print(x.length) ; null.asInstanceOf[R] } + def g(x: String): R +} + +object Test { + object bar1 extends Foo1[String] { def g(x: String): String = { print(x.length) ; "" } } + object bar2 extends Foo2[String] { def g(x: String): String = { print(x.length) ; "" } } + object bar3 extends Foo3[String] { def g(x: String): String = { print(x.length) ; "" } } + object bar4 extends Foo4[String] { def g(x: String): String = { print(x.length) ; "" } } + + // Notice that in bar5, f and g require THREE bridges, because the final + // implementation is (String)String, but: + // + // inherited abstract signatures: T(R), (T)String, and (String)R + // which erase to: (Object)Object, (Object)String, and (String)Object + // + // each of which must be bridged to the actual (String)String implementation. + // + // public java.lang.String Test$bar5$.g(java.lang.String) + // public java.lang.Object Test$bar5$.g(java.lang.String) + // public java.lang.Object Test$bar5$.g(java.lang.Object) + // public java.lang.String Test$bar5$.g(java.lang.Object) + object bar5 extends Foo1[String] with Foo2[String] { + override def f(x: String): String = { print(x.length) ; x } + def g(x: String): String = { print(x.length) ; x } + } + + final def m1[T, R](x: Base[T, R], y: T) = { x.f(y) ; x.g(y) ; x.h(y) } + final def m2[T](x: Base[T, String], y: T) = { x.f(y) ; x.g(y) ; x.h(y) } + final def m3[R](x: Base[String, R]) = { x.f("") ; x.g("") ; x.h("") } + final def m4(x: Base[String, String]) = { x.f("") ; x.g("") ; x.h("") } + + final def m11[T](x: Foo1[T], y: T) = { x.f(y) ; x.g(y) ; x.h(y) } + final def m12(x: Foo1[String]) = { x.f("") ; x.g("") ; x.h("") } + final def m21[T](x: Foo2[T], y: T) = { x.f("") ; x.g("") ; x.h("") } + final def m22(x: Foo2[String]) = { x.f("") ; x.g("") ; x.h("") } + final def m31[T](x: Foo3[T], y: T) = { x.f(y) ; x.g(y) ; x.h(y) } + final def m32(x: Foo3[String]) = { x.f("") ; x.g("") ; x.h("") } + final def m41[T](x: Foo4[T], y: T) = { x.f("") ; x.g("") ; x.h("") } + final def m42(x: Foo4[String]) = { x.f("") ; x.g("") ; x.h("") } + + def go = { + m1(bar1, "") ; m2(bar1, "") ; m3(bar1) ; m4(bar1) + m1(bar2, "") ; m2(bar2, "") ; m3(bar2) ; m4(bar2) + m1(bar3, "") ; m2(bar3, "") ; m3(bar3) ; m4(bar3) + m1(bar4, "") ; m2(bar4, "") ; m3(bar4) ; m4(bar4) + + m11(bar1, "") ; m12(bar1) + m21(bar2, "") ; m22(bar2) + m31(bar3, "") ; m32(bar3) + m41(bar4, "") ; m42(bar4) + "" + } + + def flagsString(m: java.lang.reflect.Method) = { + val str = List( + if (m.isBridge) "" else "", + if (m.isSynthetic) "" else "" + ) filterNot (_ == "") mkString " " + + if (str == "") "" else " " + str + // + // val flags = scala.reflect.internal.ClassfileConstants.toScalaMethodFlags(m.getModifiers()) + // scala.tools.nsc.symtab.Flags.flagsToString(flags) + } + + def show(clazz: Class[_]): Unit = { + print(clazz.toString + " {") + clazz.getMethods.sortBy(x => (x.getName, x.isBridge, x.toString)) filter (_.getName.length == 1) foreach { m => + print("\n " + m + flagsString(m)) + if ("" + m != "" + m.toGenericString) { + print("\n generic: " + m.toGenericString) + } + } + println("\n}") + println("") + } + def show(x: AnyRef): Unit = { show(x.getClass) } + def show(x: String): Unit = { show(Class.forName(x)) } + + def main(args: Array[String]): Unit = { + List(bar1, bar2, bar3, bar4, bar5) foreach show + List("Foo1", "Foo2") foreach show + println(go) + } +} diff --git a/tests/run/t3452b-bcode/J_2.java b/tests/run/t3452b-bcode/J_2.java deleted file mode 100644 index 839f334508e7..000000000000 --- a/tests/run/t3452b-bcode/J_2.java +++ /dev/null @@ -1,6 +0,0 @@ -public class J_2 { - public static void j() { - StringSearch.search("test"); - StringSearch.searchC("test"); - } -} diff --git a/tests/run/t3452b-bcode/S_1.scala b/tests/run/t3452b-bcode/S_1.scala deleted file mode 100644 index a209f1203539..000000000000 --- a/tests/run/t3452b-bcode/S_1.scala +++ /dev/null @@ -1,17 +0,0 @@ -trait Search[M] { - def search(input: M): C[Int] = { - println("Search received: " + input) - null - } -} - -class SearchC[M] { - def searchC(input: M): C[Int] = { - println("SearchC received: " + input) - null - } -} - -object StringSearch extends SearchC[String] with Search[String] - -trait C[T] diff --git a/tests/run/t3452b-bcode/S_3.scala b/tests/run/t3452b-bcode/S_3.scala deleted file mode 100644 index 102b433f478c..000000000000 --- a/tests/run/t3452b-bcode/S_3.scala +++ /dev/null @@ -1,5 +0,0 @@ -object Test { - def main(args: Array[String]): Unit = { - J_2.j() - } -} diff --git a/tests/run/t3452d/A.scala b/tests/run/t3452d/A.scala index 67a2080d273b..fd88a98793ab 100644 --- a/tests/run/t3452d/A.scala +++ b/tests/run/t3452d/A.scala @@ -2,6 +2,6 @@ trait TraversableLike[A, Repr] { def tail: Repr = null.asInstanceOf[Repr] } -abstract class AbstractTrav[A] extends TraversableLike[A, Traversable[A]] +abstract class AbstractTrav[A] extends TraversableLike[A, Iterable[A]] class C[A] extends AbstractTrav[A] diff --git a/tests/run/t3452d/Test.java b/tests/run/t3452d/Test.java index 875be6176c5b..16e95dbd1d30 100644 --- a/tests/run/t3452d/Test.java +++ b/tests/run/t3452d/Test.java @@ -1,12 +1,6 @@ -import scala.collection.immutable.Nil; -import scala.collection.immutable.List; -import scala.collection.Traversable; - public class Test { public static void main(String[] args) { C c = new C(); - // TODO add a bridge during mixin so we can expose - // sharper generic signature for `tail`. - /*Traversable*/ Object ls = c.tail(); + scala.collection.Iterable ls = c.tail(); } } diff --git a/tests/run/t3452g/A.scala b/tests/run/t3452g/A.scala index a3f74c1e1e4c..df151d5313b3 100644 --- a/tests/run/t3452g/A.scala +++ b/tests/run/t3452g/A.scala @@ -4,6 +4,8 @@ trait TraversableLike[A, Repr] { abstract class AbstractTrav[A] extends TraversableLike[A, AbstractTrav[A]] +class C1 extends AbstractTrav[String] + object O extends AbstractTrav[String] -class C[A] extends AbstractTrav[A] +class C2[A] extends AbstractTrav[A] diff --git a/tests/run/t3452g/Test.java b/tests/run/t3452g/Test.java index c3b4222d1686..cb21f10150de 100644 --- a/tests/run/t3452g/Test.java +++ b/tests/run/t3452g/Test.java @@ -1,14 +1,10 @@ - public class Test { - public static void main(String[] args) { - // To get better types here, we would need to - // add bridge during mixin so we can expose - // a generic return type of Traversable, because the erasure - // of this (Traversable) differs from the erasure of the mixed - // method (erasure(Repr) = Object) + public static void main(String[] args) { + AbstractTrav lsSharp1 = new C1().tail(); - Object lsSharp = O.tail(); + // Object is the result type for the static forwarder (might be because of #11305) + Object lsSharp2 = O.tail(); - Object lsSharp2 = new C().tail(); - } + AbstractTrav lsSharp3 = new C2().tail(); + } } diff --git a/tests/run/t3452h.scala b/tests/run/t3452h.scala index d61fb065ba99..6237d3ea641a 100644 --- a/tests/run/t3452h.scala +++ b/tests/run/t3452h.scala @@ -1,17 +1,8 @@ -class Mix extends Foo with Bar { f; } +class Mix___eFoo_I_wBar__f extends Foo_I_ with Bar__f { f; } trait T -abstract class Foo { - class I extends T - def f: I - f -} -trait Bar { - type I >: Null <: T - def f: I = null - f - def gobble: I = null -} +abstract class Foo_I_ { class I extends T ; def f: I ; f; } +trait Bar__f { type I>:Null<:T; def f: I = {null}; f; def gobble: I = {null}} -object Test extends dotty.runtime.LegacyApp { - new Mix +object Test extends App { + new Mix___eFoo_I_wBar__f } diff --git a/tests/run/t7932.check b/tests/run/t7932.check new file mode 100644 index 000000000000..823f1183f0e0 --- /dev/null +++ b/tests/run/t7932.check @@ -0,0 +1,6 @@ +public Category C.category() +public Category C.category1() +public default Category M1.category() +public default Category M1.category1() +public default Category M2.category() +public default Category M2.category1() diff --git a/tests/run/t7932.scala b/tests/run/t7932.scala new file mode 100644 index 000000000000..de72f3eb54f4 --- /dev/null +++ b/tests/run/t7932.scala @@ -0,0 +1,30 @@ +import scala.language.higherKinds + +class Category[M[_, _]] + +trait M1[F] { + type X[a, b] = F + def category: Category[X] = null + def category1: Category[Tuple2] = null +} + +// The second trait is needed to make sure there's a forwarder generated in C. +// otherwise the trait methods are just the inherited default methods from M1. +trait M2[F] { self: M1[F] => + override def category: Category[X] = null + override def category1: Category[Tuple2] = null +} + +abstract class C extends M1[Float] with M2[Float] + +object Test { + def t(c: Class[_]) = { + val ms = c.getMethods.filter(_.getName.startsWith("category")) + println(ms.map(_.toGenericString).sorted.mkString("\n")) + } + def main(args: Array[String]): Unit = { + t(classOf[C]) + t(classOf[M1[_]]) + t(classOf[M2[_]]) + } +} diff --git a/tests/run/t8905/DoubleRDD.scala b/tests/run/t8905/DoubleRDD.scala new file mode 100644 index 000000000000..65e993ffe02b --- /dev/null +++ b/tests/run/t8905/DoubleRDD.scala @@ -0,0 +1,9 @@ +import java.util.Comparator + +trait RDDLike[T] { + def max(comp: Comparator[T]): T = { + (1.0).asInstanceOf[T] + } +} + +class DoubleRDD extends RDDLike[java.lang.Double] { } diff --git a/tests/run/t8905/Test.java b/tests/run/t8905/Test.java new file mode 100644 index 000000000000..d1a0667cfed5 --- /dev/null +++ b/tests/run/t8905/Test.java @@ -0,0 +1,20 @@ +import java.util.Comparator; + +public class Test { + private static class DoubleComparator implements Comparator { + public int compare(Double o1, Double o2) { + return o1.compareTo(o2); + } + } + + public static void main(String[] args) { + DoubleRDD rdd = new DoubleRDD(); + RDDLike rddLike = rdd; + + // This call works fine: + double rddLikeMax = rddLike.max(new DoubleComparator()); + // In Scala 2.10.4, this code compiles but this call fails at runtime: + // java.lang.NoSuchMethodError: DoubleRDD.max(Ljava/util/Comparator;)Ljava/lang/Double; + double rddMax = rdd.max(new DoubleComparator()); + } +}