Merge pull request #1883 from dotty-staging/fix-1877

Fix #1877: Add forwarders for primitive/generic mixins.

14 files changed, 195 insertions, 6 deletions

diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala
index 900d2b0e3..1d319242b 100644
--- a/compiler/src/dotty/tools/dotc/Compiler.scala
+++ b/compiler/src/dotty/tools/dotc/Compiler.scala
@@ -72,6 +72,7 @@ class Compiler {
            new ElimByName,          // Expand by-name parameters and arguments
            new AugmentScala2Traits, // Expand traits defined in Scala 2.11 to simulate old-style rewritings
            new ResolveSuper,        // Implement super accessors and add forwarders to trait methods
+           new PrimitiveForwarders, // Add forwarders to trait methods that have a mismatch between generic and primitives
            new ArrayConstructors),  // Intercept creation of (non-generic) arrays and intrinsify.
       List(new Erasure),            // Rewrite types to JVM model, erasing all type parameters, abstract types and refinements.
       List(new ElimErasedValueType, // Expand erased value types to their underlying implmementation types
diff --git a/compiler/src/dotty/tools/dotc/transform/MixinOps.scala b/compiler/src/dotty/tools/dotc/transform/MixinOps.scala
index 6cebf7197..25aa9ffaf 100644
--- a/compiler/src/dotty/tools/dotc/transform/MixinOps.scala
+++ b/compiler/src/dotty/tools/dotc/transform/MixinOps.scala
@@ -48,11 +48,7 @@ class MixinOps(cls: ClassSymbol, thisTransform: DenotTransformer)(implicit ctx:
    *   - there are multiple traits defining method with same signature
    */
   def needsForwarder(meth: Symbol): Boolean = {
-    lazy val competingMethods = cls.baseClasses.iterator
-      .filter(_ ne meth.owner)
-      .map(meth.overriddenSymbol)
-      .filter(_.exists)
-      .toList
+    lazy val competingMethods = competingMethodsIterator(meth).toList
 
     def needsDisambiguation = competingMethods.exists(x=> !(x is Deferred)) // multiple implementations are available
     def hasNonInterfaceDefinition = competingMethods.exists(!_.owner.is(Trait)) // there is a definition originating from class
@@ -61,8 +57,38 @@ class MixinOps(cls: ClassSymbol, thisTransform: DenotTransformer)(implicit ctx:
     (needsDisambiguation || hasNonInterfaceDefinition || meth.owner.is(Scala2x))
   }
 
+  /** Get `sym` of the method that needs a forwarder
+    *  Method needs a forwarder in those cases:
+    *   - there is a trait that defines a primitive version of implemented polymorphic method.
+    *   - there is a trait that defines a polymorphic version of implemented primitive method.
+    */
+  def needsPrimitiveForwarderTo(meth: Symbol): Option[Symbol] = {
+    def hasPrimitiveMissMatch(tp1: Type, tp2: Type): Boolean = (tp1, tp2) match {
+      case (tp1: MethodicType, tp2: MethodicType) =>
+        hasPrimitiveMissMatch(tp1.resultType, tp2.resultType) ||
+        tp1.paramTypess.flatten.zip(tp1.paramTypess.flatten).exists(args => hasPrimitiveMissMatch(args._1, args._2))
+      case _ =>
+        def isPrimitiveOrValueClass(sym: Symbol): Boolean = sym.isPrimitiveValueClass || sym.isValueClass
+        isPrimitiveOrValueClass(tp1.typeSymbol) ^ isPrimitiveOrValueClass(tp2.typeSymbol)
+    }
+
+    def needsPrimitiveForwarder(m: Symbol): Boolean =
+      m.owner != cls && !m.is(Deferred) && hasPrimitiveMissMatch(meth.info, m.info)
+
+    if (!meth.is(Method | Deferred, butNot = PrivateOrAccessor) || meth.overriddenSymbol(cls).exists || needsForwarder(meth)) None
+    else competingMethodsIterator(meth).find(needsPrimitiveForwarder)
+  }
+
+  final val PrivateOrAccessor = Private | Accessor
   final val PrivateOrAccessorOrDeferred = Private | Accessor | Deferred
 
   def forwarder(target: Symbol) = (targs: List[Type]) => (vrefss: List[List[Tree]]) =>
     superRef(target).appliedToTypes(targs).appliedToArgss(vrefss)
+
+  private def competingMethodsIterator(meth: Symbol): Iterator[Symbol] = {
+    cls.baseClasses.iterator
+      .filter(_ ne meth.owner)
+      .map(meth.overriddenSymbol)
+      .filter(_.exists)
+  }
 }
diff --git a/compiler/src/dotty/tools/dotc/transform/PrimitiveForwarders.scala b/compiler/src/dotty/tools/dotc/transform/PrimitiveForwarders.scala
new file mode 100644
index 000000000..d752ce8e7
--- /dev/null
+++ b/compiler/src/dotty/tools/dotc/transform/PrimitiveForwarders.scala
@@ -0,0 +1,51 @@
+package dotty.tools.dotc
+package transform
+
+import core._
+import TreeTransforms._
+import Contexts.Context
+import Flags._
+import SymUtils._
+import Symbols._
+import SymDenotations._
+import Types._
+import Decorators._
+import DenotTransformers._
+import StdNames._
+import NameOps._
+import ast.Trees._
+import util.Positions._
+import Names._
+import collection.mutable
+import ResolveSuper._
+
+/** This phase adds forwarder where mixedin generic and primitive typed methods have a missmatch.
+  *  In particular for every method that is declared both as generic with a primitive type and with a primitive type
+  *    `<mods> def f[Ts](ps1)...(psN): U` in trait M` and
+  *    `<mods> def f[Ts](ps1)...(psN): V = ...` in implemented in N`
+  *    where U is a primitive and V a polymorphic type (or vice versa) needs:
+  *
+  *    <mods> def f[Ts](ps1)...(psN): U = super[N].f[Ts](ps1)...(psN)
+  *
+  *  IMPORTANT: When\If Valhalla happens, we'll need to move mixin before erasure and than this code will need to be rewritten
+  *  as it will instead change super-class.
+  */
+class PrimitiveForwarders extends MiniPhaseTransform with IdentityDenotTransformer { thisTransform =>
+  import ast.tpd._
+
+  override def phaseName: String = "primitiveForwarders"
+
+  override def runsAfter = Set(classOf[ResolveSuper])
+
+  override def transformTemplate(impl: Template)(implicit ctx: Context, info: TransformerInfo) = {
+    val cls = impl.symbol.owner.asClass
+    val ops = new MixinOps(cls, thisTransform)
+    import ops._
+
+    def methodPrimitiveForwarders: List[Tree] =
+      for (meth <- mixins.flatMap(_.info.decls.flatMap(needsPrimitiveForwarderTo)).distinct)
+        yield polyDefDef(implementation(meth.asTerm), forwarder(meth))
+
+    cpy.Template(impl)(body = methodPrimitiveForwarders ::: impl.body)
+  }
+}
diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala
index e718a7e60..3a301167d 100644
--- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala
+++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala
@@ -85,7 +85,7 @@ class ResolveSuper extends MiniPhaseTransform with IdentityDenotTransformer { th
   private val PrivateOrAccessorOrDeferred = Private | Accessor | Deferred
 }
 
-object ResolveSuper{
+object ResolveSuper {
   /** Returns the symbol that is accessed by a super-accessor in a mixin composition.
    *
    *  @param base       The class in which everything is mixed together
diff --git a/tests/run/mixin-primitive-on-generic-1.check b/tests/run/mixin-primitive-on-generic-1.check
new file mode 100644
index 000000000..bb101b641
--- /dev/null
+++ b/tests/run/mixin-primitive-on-generic-1.check
@@ -0,0 +1,2 @@
+true
+true
diff --git a/tests/run/mixin-primitive-on-generic-1.scala b/tests/run/mixin-primitive-on-generic-1.scala
new file mode 100644
index 000000000..470f543ce
--- /dev/null
+++ b/tests/run/mixin-primitive-on-generic-1.scala
@@ -0,0 +1,19 @@
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    println((new Foo: Baz).value1)
+    println((new Foo: Baz).value2)
+  }
+}
+
+class Foo extends Bar[Boolean](true) with Baz
+
+class Bar[T](x: T) {
+  def value1: T = x
+  def value2(): T = x
+}
+
+trait Baz {
+  def value1: Boolean
+  def value2(): Boolean
+}
diff --git a/tests/run/mixin-primitive-on-generic-2.check b/tests/run/mixin-primitive-on-generic-2.check
new file mode 100644
index 000000000..bb101b641
--- /dev/null
+++ b/tests/run/mixin-primitive-on-generic-2.check
@@ -0,0 +1,2 @@
+true
+true
diff --git a/tests/run/mixin-primitive-on-generic-2.scala b/tests/run/mixin-primitive-on-generic-2.scala
new file mode 100644
index 000000000..37e3f6035
--- /dev/null
+++ b/tests/run/mixin-primitive-on-generic-2.scala
@@ -0,0 +1,19 @@
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    println((new Foo: Bar[Boolean]).value1)
+    println((new Foo: Bar[Boolean]).value2)
+  }
+}
+
+class Foo extends Baz with Bar[Boolean]
+
+trait Bar[T] {
+  def value1: T
+  def value2(): T
+}
+
+class Baz {
+  def value1: Boolean = true
+  def value2(): Boolean = true
+}
diff --git a/tests/run/mixin-primitive-on-generic-3.check b/tests/run/mixin-primitive-on-generic-3.check
new file mode 100644
index 000000000..bb101b641
--- /dev/null
+++ b/tests/run/mixin-primitive-on-generic-3.check
@@ -0,0 +1,2 @@
+true
+true
diff --git a/tests/run/mixin-primitive-on-generic-3.scala b/tests/run/mixin-primitive-on-generic-3.scala
new file mode 100644
index 000000000..f6ff0b63e
--- /dev/null
+++ b/tests/run/mixin-primitive-on-generic-3.scala
@@ -0,0 +1,21 @@
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    println((new Foo: Baz).value)
+    println((new Foo: Qux).value)
+  }
+}
+
+class Foo extends Bar[Boolean](true) with Baz with Qux
+
+class Bar[T](x: T) {
+  def value: T = x
+}
+
+trait Baz {
+  def value: Boolean
+}
+
+trait Qux {
+  def value: Boolean
+}
diff --git a/tests/run/mixin-primitive-on-generic-4.check b/tests/run/mixin-primitive-on-generic-4.check
new file mode 100644
index 000000000..bb101b641
--- /dev/null
+++ b/tests/run/mixin-primitive-on-generic-4.check
@@ -0,0 +1,2 @@
+true
+true
diff --git a/tests/run/mixin-primitive-on-generic-4.scala b/tests/run/mixin-primitive-on-generic-4.scala
new file mode 100644
index 000000000..ddf62b92f
--- /dev/null
+++ b/tests/run/mixin-primitive-on-generic-4.scala
@@ -0,0 +1,21 @@
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    println((new Foo: Baz).value1.v)
+    println((new Foo: Baz).value2.v)
+  }
+}
+
+class Foo extends Bar[VBoolean](new VBoolean(true)) with Baz
+
+class Bar[T](x: T) {
+  def value1: T = x
+  def value2(): T = x
+}
+
+trait Baz {
+  def value1: VBoolean
+  def value2(): VBoolean
+}
+
+class VBoolean(val v: Boolean) extends AnyVal
diff --git a/tests/run/mixin-primitive-on-generic-5.check b/tests/run/mixin-primitive-on-generic-5.check
new file mode 100644
index 000000000..bb101b641
--- /dev/null
+++ b/tests/run/mixin-primitive-on-generic-5.check
@@ -0,0 +1,2 @@
+true
+true
diff --git a/tests/run/mixin-primitive-on-generic-5.scala b/tests/run/mixin-primitive-on-generic-5.scala
new file mode 100644
index 000000000..438c130de
--- /dev/null
+++ b/tests/run/mixin-primitive-on-generic-5.scala
@@ -0,0 +1,21 @@
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    println((new Foo: Bar[VBoolean]).value1.v)
+    println((new Foo: Bar[VBoolean]).value2.v)
+  }
+}
+
+class Foo extends Baz with Bar[VBoolean]
+
+trait Bar[T] {
+  def value1: T
+  def value2(): T
+}
+
+class Baz {
+  def value1: VBoolean = new VBoolean(true)
+  def value2(): VBoolean = new VBoolean(true)
+}
+
+class VBoolean(val v: Boolean) extends AnyVal