From f1f141eb04fa79bdee5a3b2aa354c488523ac199 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Mon, 24 Aug 2015 15:10:04 +0200 Subject: [PATCH 01/11] Memoize: duplicate scala2 behaviour: don't create fields for final vals. This affect initialisation order and we rely on artifacts of initialisation order in Dotty. --- src/dotty/tools/dotc/transform/Memoize.scala | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/Memoize.scala b/src/dotty/tools/dotc/transform/Memoize.scala index 63466dc461ee..a2954b4a2dd6 100644 --- a/src/dotty/tools/dotc/transform/Memoize.scala +++ b/src/dotty/tools/dotc/transform/Memoize.scala @@ -70,13 +70,23 @@ import Decorators._ lazy val field = sym.field.orElse(newField).asTerm if (sym.is(Accessor, butNot = NoFieldNeeded)) if (sym.isGetter) { + def skipBlocks(t: Tree): Tree = t match { + case Block(a, b) if a.forall(isIdempotentExpr) => skipBlocks(b) + case _ => t + } + if (sym.is(Flags.Final) && skipBlocks(tree.rhs).isInstanceOf[Literal]) + // duplicating scalac behavior: for final vals that have rhs as constant, we do not create a field + // and instead return the value. This seemingly minor optimization has huge effect on initialization + // order and the values that can be observed during superconstructor call + tree + else { var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform) if (isWildcardArg(rhs)) rhs = EmptyTree val fieldDef = transformFollowing(ValDef(field, rhs)) - val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))(ctx.withOwner(sym), info)) + val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))(ctx.withOwner(sym), info)) Thicket(fieldDef, getterDef) } - else if (sym.isSetter) { + } else if (sym.isSetter) { if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs } // this is intended as an assertion val initializer = Assign(ref(field), ref(tree.vparamss.head.head.symbol)) cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(ctx.withOwner(sym), info)) From 869bdb0503628e8665292073087b08b91399a6c9 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Mon, 24 Aug 2015 17:46:20 +0200 Subject: [PATCH 02/11] Test behaviour of final vals. --- tests/run/final-fields.check | 6 ++++++ tests/run/final-fields.scala | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/run/final-fields.check create mode 100644 tests/run/final-fields.scala diff --git a/tests/run/final-fields.check b/tests/run/final-fields.check new file mode 100644 index 000000000000..5586903e4b41 --- /dev/null +++ b/tests/run/final-fields.check @@ -0,0 +1,6 @@ +T.f1 +T.f2 +T.f3 +T.f4 +3 2 0 0 +3 diff --git a/tests/run/final-fields.scala b/tests/run/final-fields.scala new file mode 100644 index 000000000000..3a8d30c6ab5a --- /dev/null +++ b/tests/run/final-fields.scala @@ -0,0 +1,18 @@ +trait T { + + val f1: Int = {println("T.f1"); -1} + val f2: Int = {println("T.f2"); -2} + val f3: Int = {println("T.f3"); -3} + val f4: Int = {println("T.f4"); -4} + + println(s"$f1 $f2 $f3 $f4") +} + +object Test extends T { + override final val f1 = /*super.f1*/ 1 + f2 + override final val f2 = 2 + override final val f3 = {println(3); 3} + override val f4 = 4 + + def main(args: Array[String]): Unit = {} +} From 1ff1cf5393778b7dc21f765f5c8f372c05877862 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 8 Sep 2015 10:34:48 +0200 Subject: [PATCH 03/11] Follow TermRefs when constant folding A TermRef representing a constant value needs to be considered a constant when folding. --- src/dotty/tools/dotc/core/Types.scala | 8 ++++++++ src/dotty/tools/dotc/typer/ConstFold.scala | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 5fd3b81d0b32..d04da30c07a6 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -715,6 +715,14 @@ object Types { case _ => this } + /** Widen from TermRef to its underlying non-termref + * base type, while also skipping Expr types. + */ + final def widenTermRefExpr(implicit ctx: Context): Type = stripTypeVar match { + case tp: TermRef if !tp.isOverloaded => tp.underlying.widenExpr.widenTermRefExpr + case _ => this + } + /** Widen from ExprType type to its result type. * (Note: no stripTypeVar needed because TypeVar's can't refer to ExprTypes.) */ diff --git a/src/dotty/tools/dotc/typer/ConstFold.scala b/src/dotty/tools/dotc/typer/ConstFold.scala index ac1c7260bf08..68a5d05f5f26 100644 --- a/src/dotty/tools/dotc/typer/ConstFold.scala +++ b/src/dotty/tools/dotc/typer/ConstFold.scala @@ -20,16 +20,16 @@ object ConstFold { def apply(tree: Tree)(implicit ctx: Context): Tree = finish(tree) { tree match { case Apply(Select(xt, op), yt :: Nil) => - xt.tpe match { + xt.tpe.widenTermRefExpr match { case ConstantType(x) => - yt.tpe match { + yt.tpe.widenTermRefExpr match { case ConstantType(y) => foldBinop(op, x, y) case _ => null } case _ => null } case Select(xt, op) => - xt.tpe match { + xt.tpe.widenTermRefExpr match { case ConstantType(x) => foldUnop(op, x) case _ => null } @@ -42,7 +42,7 @@ object ConstFold { */ def apply(tree: Tree, pt: Type)(implicit ctx: Context): Tree = finish(apply(tree)) { - tree.tpe match { + tree.tpe.widenTermRefExpr match { case ConstantType(x) => x convertTo pt case _ => null } From 8fc5835ca13ef5ab47b29d5240d5e8d79a870b7c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 8 Sep 2015 10:37:49 +0200 Subject: [PATCH 04/11] Constant final vals need to have right hand type. Previously, a constant right hand side was not propagated to the type of a final val that implemented another val with a given type. The inherited type was used instead. This means final vals implementing abstract vals get evaluated too late. --- src/dotty/tools/dotc/typer/Namer.scala | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/dotty/tools/dotc/typer/Namer.scala b/src/dotty/tools/dotc/typer/Namer.scala index 0e8b4d8cf1a6..99119acb3275 100644 --- a/src/dotty/tools/dotc/typer/Namer.scala +++ b/src/dotty/tools/dotc/typer/Namer.scala @@ -707,18 +707,19 @@ class Namer { typer: Typer => // println(s"final inherited for $sym: ${inherited.toString}") !!! // println(s"owner = ${sym.owner}, decls = ${sym.owner.info.decls.show}") def isInline = sym.is(Final, butNot = Method) - def widenRhs(tp: Type): Type = tp match { - case tp: TermRef => widenRhs(tp.underlying) - case tp: ExprType => widenRhs(tp.resultType) + def widenRhs(tp: Type): Type = tp.widenTermRefExpr match { case tp: ConstantType if isInline => tp case _ => tp.widen.approximateUnion } val rhsCtx = ctx.addMode(Mode.InferringReturnType) - def rhsType = typedAheadExpr(mdef.rhs, rhsProto)(rhsCtx).tpe + def rhsType = typedAheadExpr(mdef.rhs, inherited orElse rhsProto)(rhsCtx).tpe def cookedRhsType = ctx.deskolemize(widenRhs(rhsType)) - def lhsType = fullyDefinedType(cookedRhsType, "right-hand side", mdef.pos) + lazy val lhsType = fullyDefinedType(cookedRhsType, "right-hand side", mdef.pos) //if (sym.name.toString == "y") println(i"rhs = $rhsType, cooked = $cookedRhsType") - if (inherited.exists) inherited + if (inherited.exists) + if (sym.is(Final, butNot = Method) && lhsType.isInstanceOf[ConstantType]) + lhsType // keep constant types that fill in for a non-constant (to be revised when inline has landed). + else inherited else { if (sym is Implicit) { val resStr = if (mdef.isInstanceOf[DefDef]) "result " else "" From 8a538ce728ed61c42ddb66f196dd217b8451bf9e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 8 Sep 2015 10:39:44 +0200 Subject: [PATCH 05/11] Move literalize functionality to PostTyper Now, PostTyper replaces constant expressions with literals. If we wait any longer then any tree rewriting of an application node would have to do constant folding again, which is a hassle. With the previous late Literalize phase, constant expressions consisting of operations and arguments lost their constantness in PostTyper. --- src/dotty/tools/dotc/Compiler.scala | 1 - .../tools/dotc/transform/PostTyper.scala | 41 +++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index e4b328a82058..c537d8885a99 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -56,7 +56,6 @@ class Compiler { List(new VCInlineMethods, new SeqLiterals, new InterceptedMethods, - new Literalize, new Getters, new ClassTags, new ElimByName, diff --git a/src/dotty/tools/dotc/transform/PostTyper.scala b/src/dotty/tools/dotc/transform/PostTyper.scala index a0670bca0814..7a25a9870f00 100644 --- a/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/src/dotty/tools/dotc/transform/PostTyper.scala @@ -68,8 +68,43 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran // TODO fill in } - private def normalizeTypeTree(tree: Tree)(implicit ctx: Context) = { - def norm(tree: Tree) = if (tree.isType) TypeTree(tree.tpe).withPos(tree.pos) else tree + /** Check bounds of AppliedTypeTrees. + * Replace type trees with TypeTree nodes. + * Replace constant expressions with Literal nodes. + * Note: Demanding idempotency instead of purityin literalize is strictly speaking too loose. + * Example + * + * object O { final val x = 42; println("43") } + * O.x + * + * Strictly speaking we can't replace `O.x` with `42`. But this would make + * most expressions non-constant. Maybe we can change the spec to accept this + * kind of eliding behavior. Or else enforce true purity in the compiler. + * The choice will be affected by what we will do with `inline` and with + * Singleton type bounds (see SIP 23). Presumably + * + * object O1 { val x: Singleton = 42; println("43") } + * object O2 { inline val x = 42; println("43") } + * + * should behave differently. + * + * O1.x should have the same effect as { println("43"; 42 } + * + * whereas + * + * O2.x = 42 + * + * Revisit this issue once we have implemented `inline`. Then we can demand + * purity of the prefix unless the selection goes to an inline val. + */ + private def normalizeTree(tree: Tree)(implicit ctx: Context): Tree = { + def literalize(tp: Type): Tree = tp.widenTermRefExpr match { + case ConstantType(value) if isIdempotentExpr(tree) => Literal(value) + case _ => tree + } + def norm(tree: Tree) = + if (tree.isType) TypeTree(tree.tpe).withPos(tree.pos) + else literalize(tree.tpe) tree match { case tree: TypeTree => tree @@ -115,7 +150,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran } override def transform(tree: Tree)(implicit ctx: Context): Tree = - try normalizeTypeTree(tree) match { + try normalizeTree(tree) match { case tree: Ident => tree.tpe match { case tpe: ThisType => This(tpe.cls).withPos(tree.pos) From ef6e2e9c6017675ffc2fefaaf2b217d05681a4c2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 8 Sep 2015 10:40:58 +0200 Subject: [PATCH 06/11] Purity checking should take constant expressions into account. A constant expression with pure arguments is pure. Previously, this was not taken into account, which meant that literalize did not work for constant expressions contiaining primitive operations or String adds. --- src/dotty/tools/dotc/ast/TreeInfo.scala | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/dotty/tools/dotc/ast/TreeInfo.scala b/src/dotty/tools/dotc/ast/TreeInfo.scala index 82c8c9d60ade..1266ebbd9658 100644 --- a/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -326,18 +326,26 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => // see a detailed explanation of this trick in `GenSymbols.reifyFreeTerm` free.symbol.hasStableFlag && isIdempotentExpr(free) */ - case Apply(fn, Nil) => + case Apply(fn, args) => + def isKnownPureOp(sym: Symbol) = + sym.owner.isPrimitiveValueClass || sym.owner == defn.StringClass // Note: After uncurry, field accesses are represented as Apply(getter, Nil), // so an Apply can also be pure. - if (fn.symbol is Stable) exprPurity(fn) else Impure + if (args.isEmpty && fn.symbol.is(Stable)) exprPurity(fn) + else if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol)) + // A constant expression with pure arguments is pure. + minOf(exprPurity(fn), args.map(exprPurity)) + else Impure case Typed(expr, _) => exprPurity(expr) case Block(stats, expr) => - (exprPurity(expr) /: stats.map(statPurity))(_ min _) + minOf(exprPurity(expr), stats.map(statPurity)) case _ => Impure } + private def minOf(l0: PurityLevel, ls: List[PurityLevel]) = (l0 /: ls)(_ min _) + def isPureExpr(tree: tpd.Tree)(implicit ctx: Context) = exprPurity(tree) == Pure def isIdempotentExpr(tree: tpd.Tree)(implicit ctx: Context) = exprPurity(tree) >= Idempotent From 56dfb4e953e42e1defd20d58fb671c6ae802e91a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 8 Sep 2015 10:41:51 +0200 Subject: [PATCH 07/11] Memoize should produce constant DefDefs for constant final vals. It produced just the right hand side literal before. --- src/dotty/tools/dotc/transform/Memoize.scala | 21 ++++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/dotty/tools/dotc/transform/Memoize.scala b/src/dotty/tools/dotc/transform/Memoize.scala index a2954b4a2dd6..53f40044f7d9 100644 --- a/src/dotty/tools/dotc/transform/Memoize.scala +++ b/src/dotty/tools/dotc/transform/Memoize.scala @@ -71,20 +71,19 @@ import Decorators._ if (sym.is(Accessor, butNot = NoFieldNeeded)) if (sym.isGetter) { def skipBlocks(t: Tree): Tree = t match { - case Block(a, b) if a.forall(isIdempotentExpr) => skipBlocks(b) + case Block(_, t1) => skipBlocks(t1) case _ => t } - if (sym.is(Flags.Final) && skipBlocks(tree.rhs).isInstanceOf[Literal]) - // duplicating scalac behavior: for final vals that have rhs as constant, we do not create a field - // and instead return the value. This seemingly minor optimization has huge effect on initialization - // order and the values that can be observed during superconstructor call - tree - else { - var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform) - if (isWildcardArg(rhs)) rhs = EmptyTree - val fieldDef = transformFollowing(ValDef(field, rhs)) + skipBlocks(tree.rhs) match { + case lit: Literal if sym.is(Final) && isIdempotentExpr(tree.rhs) => + // see remark about idempotency in PostTyper#normalizeTree + cpy.DefDef(tree)(rhs = lit) + case _ => + var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform) + if (isWildcardArg(rhs)) rhs = EmptyTree + val fieldDef = transformFollowing(ValDef(field, rhs)) val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))(ctx.withOwner(sym), info)) - Thicket(fieldDef, getterDef) + Thicket(fieldDef, getterDef) } } else if (sym.isSetter) { if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs } // this is intended as an assertion From c8b359d5a29f76f8ed7985d22a6515637befe7f8 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 8 Sep 2015 10:42:20 +0200 Subject: [PATCH 08/11] Augment test file to test for propagation of constant types. --- tests/run/final-fields.scala | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/run/final-fields.scala b/tests/run/final-fields.scala index 3a8d30c6ab5a..77c0f1bc8526 100644 --- a/tests/run/final-fields.scala +++ b/tests/run/final-fields.scala @@ -8,6 +8,24 @@ trait T { println(s"$f1 $f2 $f3 $f4") } +trait U { + val f2: Int +} + +object Test0 extends U { + final val f1 = 1 + final val f2 = 2 + final val f3 = f1 + f2 + val f4: 3 = f3 +} + +object Test1 extends U { + final val f1 = 1 + final val f3 = f1 + f2 + final val f2 = 2 + val f4: 3 = f3 +} + object Test extends T { override final val f1 = /*super.f1*/ 1 + f2 override final val f2 = 2 From 3ff309751d2fd30d1a63eac306b6d871f840b505 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 8 Sep 2015 12:26:56 +0200 Subject: [PATCH 09/11] Add another test Check that calling a side effecting function returning a constant type does not get suppressed. --- tests/run/final-fields.check | 1 + tests/run/final-fields.scala | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/run/final-fields.check b/tests/run/final-fields.check index 5586903e4b41..1b6c826871da 100644 --- a/tests/run/final-fields.check +++ b/tests/run/final-fields.check @@ -4,3 +4,4 @@ T.f3 T.f4 3 2 0 0 3 +g diff --git a/tests/run/final-fields.scala b/tests/run/final-fields.scala index 77c0f1bc8526..e951cf2f9337 100644 --- a/tests/run/final-fields.scala +++ b/tests/run/final-fields.scala @@ -24,13 +24,17 @@ object Test1 extends U { final val f3 = f1 + f2 final val f2 = 2 val f4: 3 = f3 + + } object Test extends T { override final val f1 = /*super.f1*/ 1 + f2 override final val f2 = 2 override final val f3 = {println(3); 3} - override val f4 = 4 + override val f4 = f3 + 1 + def g: 3 = { println("g"); 3 } + final val x = g + 1 def main(args: Array[String]): Unit = {} } From ea6de3869d2cee752f9a616b17ef4bb45a6288d1 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Mon, 14 Sep 2015 13:40:57 +0200 Subject: [PATCH 10/11] Memoize: bring back comment about how final vals are compiled --- src/dotty/tools/dotc/transform/Memoize.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/dotty/tools/dotc/transform/Memoize.scala b/src/dotty/tools/dotc/transform/Memoize.scala index 53f40044f7d9..fbf8ed76374c 100644 --- a/src/dotty/tools/dotc/transform/Memoize.scala +++ b/src/dotty/tools/dotc/transform/Memoize.scala @@ -76,6 +76,10 @@ import Decorators._ } skipBlocks(tree.rhs) match { case lit: Literal if sym.is(Final) && isIdempotentExpr(tree.rhs) => + // duplicating scalac behavior: for final vals that have rhs as constant, we do not create a field + // and instead return the value. This seemingly minor optimization has huge effect on initialization + // order and the values that can be observed during superconstructor call + // see remark about idempotency in PostTyper#normalizeTree cpy.DefDef(tree)(rhs = lit) case _ => From e491cfbc14ec36afccc80a589f7bc18d62f37b2f Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Mon, 14 Sep 2015 13:41:29 +0200 Subject: [PATCH 11/11] final-fields.scala: tes objects with constant final vals. --- tests/run/final-fields.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/run/final-fields.scala b/tests/run/final-fields.scala index e951cf2f9337..72447d4780cf 100644 --- a/tests/run/final-fields.scala +++ b/tests/run/final-fields.scala @@ -36,5 +36,8 @@ object Test extends T { def g: 3 = { println("g"); 3 } final val x = g + 1 - def main(args: Array[String]): Unit = {} + def main(args: Array[String]): Unit = { + Test0 + Test1 + } }