From 988f9eebdd4d13a351aad06f2cde853f29f5b550 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 10 Jan 2017 14:37:20 +0100 Subject: [PATCH 1/3] Revert "Fix #1878: Generate fields for final vars." This reverts commit 63d68bf4d3cbac82f6d9faf19acd5589603a17ee. --- .../dotty/tools/dotc/transform/Memoize.scala | 2 +- tests/run/final-var.check | 4 ---- tests/run/final-var.scala | 20 ------------------- 3 files changed, 1 insertion(+), 25 deletions(-) delete mode 100644 tests/run/final-var.check delete mode 100644 tests/run/final-var.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index 63edc0256c1d..0314d4ec4b10 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -102,7 +102,7 @@ import Decorators._ case _ => t } skipBlocks(tree.rhs) match { - case lit: Literal if sym.is(Final, butNot = Mutable) && isIdempotentExpr(tree.rhs) => + 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 diff --git a/tests/run/final-var.check b/tests/run/final-var.check deleted file mode 100644 index 7565230c01f7..000000000000 --- a/tests/run/final-var.check +++ /dev/null @@ -1,4 +0,0 @@ -false -true -false -true diff --git a/tests/run/final-var.scala b/tests/run/final-var.scala deleted file mode 100644 index 94a6c453cbda..000000000000 --- a/tests/run/final-var.scala +++ /dev/null @@ -1,20 +0,0 @@ -object Test { - def main(args: Array[String]): Unit = { - println(Obj.myFinalVar) - Obj.myFinalVar = true - println(Obj.myFinalVar) - - val o = new Cls - println(o.myFinalVar) - o.myFinalVar = true - println(o.myFinalVar) - } -} - -object Obj { - final var myFinalVar: Boolean = false -} - -class Cls { - final var myFinalVar: Boolean = false -} From 4d7b2479e85e9f674056e8a243c07675fc9fa51f Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 10 Jan 2017 14:42:01 +0100 Subject: [PATCH 2/3] Stop emitting fields for inlined fields. --- compiler/src/dotty/tools/dotc/transform/Memoize.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index 0314d4ec4b10..3df9b0daa664 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -95,6 +95,8 @@ import Decorators._ def adaptToField(tree: Tree) = if (tree.isEmpty) tree else tree.ensureConforms(field.info.widen) + val NoFieldNeeded = Lazy | Deferred | JavaDefined | (if (ctx.settings.YnoInline.value) EmptyFlags else Inline) + if (sym.is(Accessor, butNot = NoFieldNeeded)) if (sym.isGetter) { def skipBlocks(t: Tree): Tree = t match { @@ -127,5 +129,4 @@ import Decorators._ // neither getters nor setters else tree } - private val NoFieldNeeded = Lazy | Deferred | JavaDefined } From 1cb50ca13640e3e181d8e7d9b47029a6099a81d6 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 10 Jan 2017 14:45:02 +0100 Subject: [PATCH 3/3] Fix #1878: Use Inline on final vals. --- .../dotty/tools/dotc/transform/Memoize.scala | 25 ++++--------------- .../src/dotty/tools/dotc/typer/Typer.scala | 22 ++++++++++++++++ tests/run/final-var.check | 4 +++ tests/run/final-var.scala | 20 +++++++++++++++ 4 files changed, 51 insertions(+), 20 deletions(-) create mode 100644 tests/run/final-var.check create mode 100644 tests/run/final-var.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index 3df9b0daa664..8b5ceb0aadbb 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -99,26 +99,11 @@ import Decorators._ if (sym.is(Accessor, butNot = NoFieldNeeded)) if (sym.isGetter) { - def skipBlocks(t: Tree): Tree = t match { - case Block(_, t1) => skipBlocks(t1) - case _ => t - } - 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 _ => - var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform) - if (isWildcardArg(rhs)) rhs = EmptyTree - - val fieldDef = transformFollowing(ValDef(field, adaptToField(rhs))) - val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))(ctx.withOwner(sym), info)) - Thicket(fieldDef, getterDef) - } + var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform) + if (isWildcardArg(rhs)) rhs = EmptyTree + val fieldDef = transformFollowing(ValDef(field, adaptToField(rhs))) + val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))(ctx.withOwner(sym), info)) + Thicket(fieldDef, getterDef) } else if (sym.isSetter) { if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs } // this is intended as an assertion field.setFlag(Mutable) // necessary for vals mixed in from Scala2 traits diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index b70a51d1e3a4..a053a0b0d2d6 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1173,6 +1173,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit if (sym.is(Inline, butNot = DeferredOrParamAccessor)) checkInlineConformant(rhs1, em"right-hand side of inline $sym") patchIfLazy(vdef1) + patchFinalVals(vdef1) vdef1 } @@ -1185,6 +1186,27 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit patch(Position(toUntyped(vdef).pos.start), "@volatile ") } + /** Adds inline to final vals with idempotent 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 + */ + private def patchFinalVals(vdef: ValDef)(implicit ctx: Context): Unit = { + def isFinalInlinableVal(sym: Symbol): Boolean = { + sym.is(Final, butNot = Mutable) && + isIdempotentExpr(vdef.rhs) /* && + ctx.scala2Mode (stay compatible with Scala2 for now) */ + } + val sym = vdef.symbol + sym.info match { + case info: ConstantType if isFinalInlinableVal(sym) && !ctx.settings.YnoInline.value => sym.setFlag(Inline) + case _ => + } + } + def typedDefDef(ddef: untpd.DefDef, sym: Symbol)(implicit ctx: Context) = track("typedDefDef") { val DefDef(name, tparams, vparamss, tpt, _) = ddef completeAnnotations(ddef, sym) diff --git a/tests/run/final-var.check b/tests/run/final-var.check new file mode 100644 index 000000000000..7565230c01f7 --- /dev/null +++ b/tests/run/final-var.check @@ -0,0 +1,4 @@ +false +true +false +true diff --git a/tests/run/final-var.scala b/tests/run/final-var.scala new file mode 100644 index 000000000000..94a6c453cbda --- /dev/null +++ b/tests/run/final-var.scala @@ -0,0 +1,20 @@ +object Test { + def main(args: Array[String]): Unit = { + println(Obj.myFinalVar) + Obj.myFinalVar = true + println(Obj.myFinalVar) + + val o = new Cls + println(o.myFinalVar) + o.myFinalVar = true + println(o.myFinalVar) + } +} + +object Obj { + final var myFinalVar: Boolean = false +} + +class Cls { + final var myFinalVar: Boolean = false +}