From 4b48243a24e533ba81149df956fdc16cdaf1460e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 25 Jun 2015 22:03:01 +0200 Subject: [PATCH 1/6] Fix erasure of Thistypes. Thistypes erased to the underlying class. This is wrong. When seen as part of some other type, a ThisType has to erase to the erasure of the underlying type (i.e. the erasure if the selftype of the class). unittest-collections.scala failed with a MethodNotFound error because the erasure was computed incorrectly. On the other hand, a tree with a ThisType type, should keep the type, analogous to a tree with a TermRef type. --- src/dotty/tools/dotc/core/TypeErasure.scala | 6 +----- src/dotty/tools/dotc/transform/Erasure.scala | 1 + tests/{pending => }/run/unittest_collection.check | 0 tests/{pending => }/run/unittest_collection.scala | 10 +++++----- 4 files changed, 7 insertions(+), 10 deletions(-) rename tests/{pending => }/run/unittest_collection.check (100%) rename tests/{pending => }/run/unittest_collection.scala (86%) diff --git a/src/dotty/tools/dotc/core/TypeErasure.scala b/src/dotty/tools/dotc/core/TypeErasure.scala index abe5418d4424..8abab9155ad8 100644 --- a/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/src/dotty/tools/dotc/core/TypeErasure.scala @@ -315,12 +315,8 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean val parent = tp.parent if (parent isRef defn.ArrayClass) eraseArray(tp) else this(parent) - case tp: TermRef => + case _: TermRef | _: ThisType => this(tp.widen) - case tp: ThisType => - def thisTypeErasure(tpToErase: Type) = - erasureFn(isJava, semiEraseVCs = false, isConstructor, wildcardOK)(tpToErase) - thisTypeErasure(tp.cls.typeRef) case SuperType(thistpe, supertpe) => SuperType(this(thistpe), this(supertpe)) case ExprType(rt) => diff --git a/src/dotty/tools/dotc/transform/Erasure.scala b/src/dotty/tools/dotc/transform/Erasure.scala index ad261c16be71..663c309c07c1 100644 --- a/src/dotty/tools/dotc/transform/Erasure.scala +++ b/src/dotty/tools/dotc/transform/Erasure.scala @@ -271,6 +271,7 @@ object Erasure extends TypeTestsCasts{ def erasedType(tree: untpd.Tree, semiEraseVCs: Boolean)(implicit ctx: Context): Type = tree.typeOpt match { case tp: TermRef if tree.isTerm => erasedRef(tp) + case tp: ThisType => tp case tp => erasure(tp, semiEraseVCs) } diff --git a/tests/pending/run/unittest_collection.check b/tests/run/unittest_collection.check similarity index 100% rename from tests/pending/run/unittest_collection.check rename to tests/run/unittest_collection.check diff --git a/tests/pending/run/unittest_collection.scala b/tests/run/unittest_collection.scala similarity index 86% rename from tests/pending/run/unittest_collection.scala rename to tests/run/unittest_collection.scala index d10845475b74..5db9b56d2bdc 100644 --- a/tests/pending/run/unittest_collection.scala +++ b/tests/run/unittest_collection.scala @@ -4,11 +4,11 @@ object Test { def main(args: Array[String]): Unit = { test(collection.mutable.ArrayBuffer[String]()) - test(collection.mutable.ListBuffer[String]()) - class BBuf(z:ListBuffer[String]) extends BufferProxy[String] { - def self = z - } - test(new BBuf(collection.mutable.ListBuffer[String]())) +// test(collection.mutable.ListBuffer[String]()) +// class BBuf(z:ListBuffer[String]) extends BufferProxy[String] { +// def self = z +// } +// test(new BBuf(collection.mutable.ListBuffer[String]())) } def test(x: Buffer[String]): Unit = { From aac4c28e4358bd6ff55cf5391508a9196cfb36ad Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 25 Jun 2015 22:50:07 +0200 Subject: [PATCH 2/6] Remove check file. The file consisted of just a deprecation warning. Not sure what was deprecated; neither dotty nor scalac find anything wrong with it. --- tests/run/unittest_collection.check | 1 - 1 file changed, 1 deletion(-) delete mode 100644 tests/run/unittest_collection.check diff --git a/tests/run/unittest_collection.check b/tests/run/unittest_collection.check deleted file mode 100644 index df1629dd7eb1..000000000000 --- a/tests/run/unittest_collection.check +++ /dev/null @@ -1 +0,0 @@ -warning: there was one deprecation warning; re-run with -deprecation for details From 830a66276c40d9552f0d55b1b560a85a26d0b56c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jun 2015 09:53:30 +0200 Subject: [PATCH 3/6] Simplify logic in erasure. There's one behavioral change: on typedSelectFromTypeTree, we use erasedType as for a notmal ref. Before semiEraseVCs was always set to false here. I don't see how the treatment should be different. E.g. it should not matter if we see a x.y or T#y --- src/dotty/tools/dotc/transform/Erasure.scala | 27 ++++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/dotty/tools/dotc/transform/Erasure.scala b/src/dotty/tools/dotc/transform/Erasure.scala index 663c309c07c1..8446e26262dd 100644 --- a/src/dotty/tools/dotc/transform/Erasure.scala +++ b/src/dotty/tools/dotc/transform/Erasure.scala @@ -268,36 +268,29 @@ object Erasure extends TypeTestsCasts{ class Typer extends typer.ReTyper with NoChecking { import Boxing._ - def erasedType(tree: untpd.Tree, semiEraseVCs: Boolean)(implicit ctx: Context): Type = - tree.typeOpt match { - case tp: TermRef if tree.isTerm => erasedRef(tp) - case tp: ThisType => tp - case tp => erasure(tp, semiEraseVCs) - } + def erasedType(tree: untpd.Tree)(implicit ctx: Context): Type = { + val tp = tree.typeOpt + if (tree.isTerm) erasedRef(tp) else erasure(tp, semiEraseVCs = true) + } - def promote(tree: untpd.Tree, semiEraseVCs: Boolean)(implicit ctx: Context): tree.ThisTree[Type] = { + override def promote(tree: untpd.Tree)(implicit ctx: Context): tree.ThisTree[Type] = { assert(tree.hasType) - val erased = erasedType(tree, semiEraseVCs) + val erased = erasedType(tree) ctx.log(s"promoting ${tree.show}: ${erased.showWithUnderlying()}") tree.withType(erased) } - override def promote(tree: untpd.Tree)(implicit ctx: Context): tree.ThisTree[Type] = { - promote(tree, true) - } - /** When erasing most TypeTrees we should not semi-erase value types. * This is not the case for [[DefDef#tpt]], [[ValDef#tpt]] and [[Typed#tpt]], they * are handled separately by [[typedDefDef]], [[typedValDef]] and [[typedTyped]]. */ - override def typedTypeTree(tree: untpd.TypeTree, pt: Type)(implicit ctx: Context): TypeTree = { - promote(tree, semiEraseVCs = false) - } + override def typedTypeTree(tree: untpd.TypeTree, pt: Type)(implicit ctx: Context): TypeTree = + tree.withType(erasure(tree.tpe, semiEraseVCs = false)) /** This override is only needed to semi-erase type ascriptions */ override def typedTyped(tree: untpd.Typed, pt: Type)(implicit ctx: Context): Tree = { val Typed(expr, tpt) = tree - val tpt1 = promote(tpt, semiEraseVCs = true) + val tpt1 = promote(tpt) val expr1 = typed(expr, tpt1.tpe) assignType(untpd.cpy.Typed(tree)(expr1, tpt1), tpt1) } @@ -385,7 +378,7 @@ object Erasure extends TypeTestsCasts{ } override def typedSelectFromTypeTree(tree: untpd.SelectFromTypeTree, pt: Type)(implicit ctx: Context) = - untpd.Ident(tree.name).withPos(tree.pos).withType(erasedType(tree, semiEraseVCs = false)) + untpd.Ident(tree.name).withPos(tree.pos).withType(erasedType(tree)) override def typedThis(tree: untpd.This)(implicit ctx: Context): Tree = if (tree.symbol == ctx.owner.enclosingClass || tree.symbol.isStaticOwner) promote(tree) From 8017c54da7b512478a78367f9b31a6b13260b311 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jun 2015 14:19:01 +0200 Subject: [PATCH 4/6] Disable checkConstraintClosed by default Checking that constraints are closed caused cyclic reference exceptions in DottyBackedInterface. What's worrying is that these were seemingly not checked by the checkin tests. Or maybe there is some dependcy on compilation order that triggers the erros only in my setup. --- src/dotty/tools/dotc/config/Config.scala | 12 ++++++++++-- src/dotty/tools/dotc/core/TyperState.scala | 2 +- src/dotty/tools/dotc/core/Types.scala | 2 ++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/dotty/tools/dotc/config/Config.scala b/src/dotty/tools/dotc/config/Config.scala index 9e9974bdce85..97893647c7fb 100644 --- a/src/dotty/tools/dotc/config/Config.scala +++ b/src/dotty/tools/dotc/config/Config.scala @@ -32,8 +32,16 @@ object Config { */ final val checkConstraintsPropagated = false - /** Check that constraints of globally committable typer states are closed */ - final val checkConstraintsClosed = true + /** Check that constraints of globally committable typer states are closed. + * NOTE: When enabled, the check can cause CyclicReference errors because + * it traverses all elements of a type. Such failures were observed when + * compiling all of dotty together (source seems to be in GenBCode which + * accesses javac's settings.) + * + * It is recommended to turn this option on only when chasing down + * a PolyParam instantiation error. See comment in Types.TypeVar.instantiate. + */ + final val debugCheckConstraintsClosed = false /** Check that no type appearing as the info of a SymDenotation contains * skolem types. diff --git a/src/dotty/tools/dotc/core/TyperState.scala b/src/dotty/tools/dotc/core/TyperState.scala index ba7a6a8063fd..5617f568a2a7 100644 --- a/src/dotty/tools/dotc/core/TyperState.scala +++ b/src/dotty/tools/dotc/core/TyperState.scala @@ -87,7 +87,7 @@ extends TyperState(r) { override def constraint = myConstraint override def constraint_=(c: Constraint)(implicit ctx: Context) = { - if (Config.checkConstraintsClosed && isGlobalCommittable) c.checkClosed() + if (Config.debugCheckConstraintsClosed && isGlobalCommittable) c.checkClosed() myConstraint = c } diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 1270466e9cb1..d6bb9c3c574e 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -2460,6 +2460,8 @@ object Types { if (ctx.typerState.isGlobalCommittable) assert(!inst.isInstanceOf[PolyParam], i"bad inst $this := $inst, constr = ${ctx.typerState.constraint}") + // If this fails, you might want to turn on Config.debugCheckConstraintsClosed + // to help find the root of the problem. instantiateWith(inst) } From 28751a117f32fb163b54e36e20915fd6f2cc39ff Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jun 2015 14:23:34 +0200 Subject: [PATCH 5/6] Get rid of semiEraseVCs boolean parameters Replace by the pair of methods erasure/valueErasure. The boolean parameter is still kept, but only as a confuration parameter of the erasure objects. --- src/dotty/tools/dotc/core/TypeErasure.scala | 21 ++++++++++++------- src/dotty/tools/dotc/transform/Erasure.scala | 6 +++--- .../dotc/transform/ExtensionMethods.scala | 4 ++-- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/dotty/tools/dotc/core/TypeErasure.scala b/src/dotty/tools/dotc/core/TypeErasure.scala index 8abab9155ad8..9b41eb982a5e 100644 --- a/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/src/dotty/tools/dotc/core/TypeErasure.scala @@ -110,15 +110,20 @@ object TypeErasure { private def erasureCtx(implicit ctx: Context) = if (ctx.erasedTypes) ctx.withPhase(ctx.erasurePhase).addMode(Mode.FutureDefsOK) else ctx - /** The standard erasure of a Scala type. + /** The standard erasure of a Scala type. Value classes are erased as normal classes. + * + * @param tp The type to erase. + */ + def erasure(tp: Type)(implicit ctx: Context): Type = + erasureFn(isJava = false, semiEraseVCs = false, isConstructor = false, wildcardOK = false)(tp)(erasureCtx) + + /** The value class erasure of a Scala type, where value classes are semi-erased to + * ErasedValueType (they will be fully erased in [[ElimErasedValueType]]). * * @param tp The type to erase. - * @param semiEraseVCs If true, value classes are semi-erased to ErasedValueType - * (they will be fully erased in [[ElimErasedValueType]]). - * If false, they are erased like normal classes. */ - def erasure(tp: Type, semiEraseVCs: Boolean = false)(implicit ctx: Context): Type = - erasureFn(isJava = false, semiEraseVCs, isConstructor = false, wildcardOK = false)(tp)(erasureCtx) + def valueErasure(tp: Type)(implicit ctx: Context): Type = + erasureFn(isJava = false, semiEraseVCs = true, isConstructor = false, wildcardOK = false)(tp)(erasureCtx) def sigName(tp: Type, isJava: Boolean)(implicit ctx: Context): TypeName = { val seqClass = if (isJava) defn.ArrayClass else defn.SeqClass @@ -141,7 +146,7 @@ object TypeErasure { case tp: ThisType => tp case tp => - erasure(tp, semiEraseVCs = true) + valueErasure(tp) } /** The symbol's erased info. This is the type's erasure, except for the following symbols: @@ -392,7 +397,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean private def eraseDerivedValueClassRef(tref: TypeRef)(implicit ctx: Context): Type = { val cls = tref.symbol.asClass val underlying = underlyingOfValueClass(cls) - ErasedValueType(cls, erasure(underlying, semiEraseVCs = true)) + ErasedValueType(cls, valueErasure(underlying)) } diff --git a/src/dotty/tools/dotc/transform/Erasure.scala b/src/dotty/tools/dotc/transform/Erasure.scala index 8446e26262dd..8b4c6a87db19 100644 --- a/src/dotty/tools/dotc/transform/Erasure.scala +++ b/src/dotty/tools/dotc/transform/Erasure.scala @@ -270,7 +270,7 @@ object Erasure extends TypeTestsCasts{ def erasedType(tree: untpd.Tree)(implicit ctx: Context): Type = { val tp = tree.typeOpt - if (tree.isTerm) erasedRef(tp) else erasure(tp, semiEraseVCs = true) + if (tree.isTerm) erasedRef(tp) else valueErasure(tp) } override def promote(tree: untpd.Tree)(implicit ctx: Context): tree.ThisTree[Type] = { @@ -285,7 +285,7 @@ object Erasure extends TypeTestsCasts{ * are handled separately by [[typedDefDef]], [[typedValDef]] and [[typedTyped]]. */ override def typedTypeTree(tree: untpd.TypeTree, pt: Type)(implicit ctx: Context): TypeTree = - tree.withType(erasure(tree.tpe, semiEraseVCs = false)) + tree.withType(erasure(tree.tpe)) /** This override is only needed to semi-erase type ascriptions */ override def typedTyped(tree: untpd.Typed, pt: Type)(implicit ctx: Context): Tree = { @@ -454,7 +454,7 @@ object Erasure extends TypeTestsCasts{ if (pt.isValueType) pt else { if (tree.typeOpt.derivesFrom(ctx.definitions.UnitClass)) tree.typeOpt - else erasure(tree.typeOpt, semiEraseVCs = true) + else valueErasure(tree.typeOpt) } } diff --git a/src/dotty/tools/dotc/transform/ExtensionMethods.scala b/src/dotty/tools/dotc/transform/ExtensionMethods.scala index 503016d8bab2..f50e3324dc0e 100644 --- a/src/dotty/tools/dotc/transform/ExtensionMethods.scala +++ b/src/dotty/tools/dotc/transform/ExtensionMethods.scala @@ -14,7 +14,7 @@ import core._ import Phases.Phase import Types._, Contexts._, Constants._, Names._, NameOps._, Flags._, DenotTransformers._ import SymDenotations._, Symbols._, StdNames._, Annotations._, Trees._, Scopes._, Denotations._ -import TypeErasure.{ erasure, ErasedValueType } +import TypeErasure.{ erasure, valueErasure, ErasedValueType } import TypeUtils._ import util.Positions._ import Decorators._ @@ -65,7 +65,7 @@ class ExtensionMethods extends MiniPhaseTransform with DenotTransformer with Ful } } - val underlying = erasure(underlyingOfValueClass(valueClass), semiEraseVCs = true) + val underlying = valueErasure(underlyingOfValueClass(valueClass)) val evt = ErasedValueType(valueClass, underlying) val u2evtSym = ctx.newSymbol(moduleSym, nme.U2EVT, Synthetic | Method, MethodType(List(nme.x_0), List(underlying), evt)) From ef0184826b784fda9e2e1ef9aab31cab692cf3d2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jun 2015 14:29:49 +0200 Subject: [PATCH 6/6] Restored full test Uncommented parts that were left accidentally commented out when debugging. --- tests/run/unittest_collection.scala | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/run/unittest_collection.scala b/tests/run/unittest_collection.scala index 5db9b56d2bdc..d10845475b74 100644 --- a/tests/run/unittest_collection.scala +++ b/tests/run/unittest_collection.scala @@ -4,11 +4,11 @@ object Test { def main(args: Array[String]): Unit = { test(collection.mutable.ArrayBuffer[String]()) -// test(collection.mutable.ListBuffer[String]()) -// class BBuf(z:ListBuffer[String]) extends BufferProxy[String] { -// def self = z -// } -// test(new BBuf(collection.mutable.ListBuffer[String]())) + test(collection.mutable.ListBuffer[String]()) + class BBuf(z:ListBuffer[String]) extends BufferProxy[String] { + def self = z + } + test(new BBuf(collection.mutable.ListBuffer[String]())) } def test(x: Buffer[String]): Unit = {