From 9f505a42a1c100eedab9748321a77d4bc345af61 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 6 Jan 2017 10:58:12 +0100 Subject: Fix #1877: Add forwarders for primitive/generic mixins. --- .../src/dotty/tools/dotc/transform/MixinOps.scala | 35 ++++++++++++++++++---- .../dotty/tools/dotc/transform/ResolveSuper.scala | 14 ++++++++- tests/run/mixin-primitive-on-generic-1.check | 2 ++ tests/run/mixin-primitive-on-generic-1.scala | 19 ++++++++++++ tests/run/mixin-primitive-on-generic-2.check | 2 ++ tests/run/mixin-primitive-on-generic-2.scala | 19 ++++++++++++ tests/run/mixin-primitive-on-generic-3.check | 2 ++ tests/run/mixin-primitive-on-generic-3.scala | 21 +++++++++++++ 8 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 tests/run/mixin-primitive-on-generic-1.check create mode 100644 tests/run/mixin-primitive-on-generic-1.scala create mode 100644 tests/run/mixin-primitive-on-generic-2.check create mode 100644 tests/run/mixin-primitive-on-generic-2.scala create mode 100644 tests/run/mixin-primitive-on-generic-3.check create mode 100644 tests/run/mixin-primitive-on-generic-3.scala diff --git a/compiler/src/dotty/tools/dotc/transform/MixinOps.scala b/compiler/src/dotty/tools/dotc/transform/MixinOps.scala index 6cebf7197..c39e72b45 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,37 @@ 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 _ => + tp1.typeSymbol.isPrimitiveValueClass ^ tp2.typeSymbol.isPrimitiveValueClass + } + + 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/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index e718a7e60..c4d8f5e33 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -38,6 +38,14 @@ import ResolveSuper._ * * def f[Ts](ps1)...(psN): U = super[M].f[Ts](ps1)...(psN) * + * 3.3 (done in `methodPrimitiveForwarders`) For every method that is declared both + * as generic with a primitive type and with a primitive type + * ` def f[Ts](ps1)...(psN): U` in trait M` and + * ` def f[Ts](ps1)...(psN): V = ...` in implemented in N` + * where U is a primitive and V a polymorphic type (or vice versa) needs: + * + * def f[Ts](ps1)...(psN): U = super[N].f[Ts](ps1)...(psN) + * * A method in M needs to be disambiguated if it is concrete, not overridden in C, * and if it overrides another concrete method. * @@ -65,7 +73,11 @@ class ResolveSuper extends MiniPhaseTransform with IdentityDenotTransformer { th for (meth <- mixin.info.decls.toList if needsForwarder(meth)) yield polyDefDef(implementation(meth.asTerm), forwarder(meth)) - val overrides = mixins.flatMap(mixin => superAccessors(mixin) ::: methodOverrides(mixin)) + def methodPrimitiveForwarders: List[Tree] = + for (meth <- mixins.flatMap(_.info.decls.flatMap(needsPrimitiveForwarderTo)).distinct) + yield polyDefDef(implementation(meth.asTerm), forwarder(meth)) + + val overrides = mixins.flatMap(mixin => superAccessors(mixin) ::: methodOverrides(mixin)) ::: methodPrimitiveForwarders cpy.Template(impl)(body = overrides ::: impl.body) } 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 +} -- cgit v1.2.3 From cf57534c6141593fbe2ac2fec9d101fa662ecf05 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Wed, 11 Jan 2017 18:41:58 +0100 Subject: Add PrimitiveForwarders and fix forwarding on value classes. --- compiler/src/dotty/tools/dotc/Compiler.scala | 1 + .../src/dotty/tools/dotc/transform/MixinOps.scala | 3 +- .../tools/dotc/transform/PrimitiveForwarders.scala | 51 ++++++++++++++++++++++ .../dotty/tools/dotc/transform/ResolveSuper.scala | 16 +------ tests/run/mixin-primitive-on-generic-4.check | 2 + tests/run/mixin-primitive-on-generic-4.scala | 21 +++++++++ tests/run/mixin-primitive-on-generic-5.check | 2 + tests/run/mixin-primitive-on-generic-5.scala | 21 +++++++++ 8 files changed, 102 insertions(+), 15 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/transform/PrimitiveForwarders.scala create mode 100644 tests/run/mixin-primitive-on-generic-4.check create mode 100644 tests/run/mixin-primitive-on-generic-4.scala create mode 100644 tests/run/mixin-primitive-on-generic-5.check create mode 100644 tests/run/mixin-primitive-on-generic-5.scala 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 c39e72b45..25aa9ffaf 100644 --- a/compiler/src/dotty/tools/dotc/transform/MixinOps.scala +++ b/compiler/src/dotty/tools/dotc/transform/MixinOps.scala @@ -68,7 +68,8 @@ class MixinOps(cls: ClassSymbol, thisTransform: DenotTransformer)(implicit ctx: hasPrimitiveMissMatch(tp1.resultType, tp2.resultType) || tp1.paramTypess.flatten.zip(tp1.paramTypess.flatten).exists(args => hasPrimitiveMissMatch(args._1, args._2)) case _ => - tp1.typeSymbol.isPrimitiveValueClass ^ tp2.typeSymbol.isPrimitiveValueClass + def isPrimitiveOrValueClass(sym: Symbol): Boolean = sym.isPrimitiveValueClass || sym.isValueClass + isPrimitiveOrValueClass(tp1.typeSymbol) ^ isPrimitiveOrValueClass(tp2.typeSymbol) } def needsPrimitiveForwarder(m: Symbol): Boolean = 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 + * ` def f[Ts](ps1)...(psN): U` in trait M` and + * ` def f[Ts](ps1)...(psN): V = ...` in implemented in N` + * where U is a primitive and V a polymorphic type (or vice versa) needs: + * + * 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 c4d8f5e33..3a301167d 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -38,14 +38,6 @@ import ResolveSuper._ * * def f[Ts](ps1)...(psN): U = super[M].f[Ts](ps1)...(psN) * - * 3.3 (done in `methodPrimitiveForwarders`) For every method that is declared both - * as generic with a primitive type and with a primitive type - * ` def f[Ts](ps1)...(psN): U` in trait M` and - * ` def f[Ts](ps1)...(psN): V = ...` in implemented in N` - * where U is a primitive and V a polymorphic type (or vice versa) needs: - * - * def f[Ts](ps1)...(psN): U = super[N].f[Ts](ps1)...(psN) - * * A method in M needs to be disambiguated if it is concrete, not overridden in C, * and if it overrides another concrete method. * @@ -73,11 +65,7 @@ class ResolveSuper extends MiniPhaseTransform with IdentityDenotTransformer { th for (meth <- mixin.info.decls.toList if needsForwarder(meth)) yield polyDefDef(implementation(meth.asTerm), forwarder(meth)) - def methodPrimitiveForwarders: List[Tree] = - for (meth <- mixins.flatMap(_.info.decls.flatMap(needsPrimitiveForwarderTo)).distinct) - yield polyDefDef(implementation(meth.asTerm), forwarder(meth)) - - val overrides = mixins.flatMap(mixin => superAccessors(mixin) ::: methodOverrides(mixin)) ::: methodPrimitiveForwarders + val overrides = mixins.flatMap(mixin => superAccessors(mixin) ::: methodOverrides(mixin)) cpy.Template(impl)(body = overrides ::: impl.body) } @@ -97,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-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 -- cgit v1.2.3