From 1f92d31e13dd4e5e7ba2ac4bf23776e43fc3898d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 18 Oct 2019 13:11:47 +0200 Subject: [PATCH 1/3] Fix #7434: Tighten condition for optimizing synthetic objects in LazyVal --- .../src/dotty/tools/dotc/transform/LazyVals.scala | 6 +++++- tests/run/t704.check | 6 ++++++ tests/{pos => run}/t704.scala | 11 +++++++++-- 3 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 tests/run/t704.check rename tests/{pos => run}/t704.scala (60%) diff --git a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala index 2802bae99fb3..d7dcd6f3db55 100644 --- a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala @@ -73,7 +73,11 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer { else { val isField = sym.owner.isClass if (isField) - if (sym.isAllOf(SyntheticModule)) + if sym.isAllOf(SyntheticModule) && sym.allOverriddenSymbols.isEmpty then + // I am not sure what the conditions for this optimization should be. + // It was applied for all synthetic objects, but this is clearly false, as t704 demonstrates. + // It seems we have to at least exclude synthetic objects that derive from mixins. + // This is done by demanding that the object does not override anything. transformSyntheticModule(tree) else if (sym.isThreadUnsafe || ctx.settings.scalajs.value) if (sym.is(Module) && !ctx.settings.scalajs.value) { diff --git a/tests/run/t704.check b/tests/run/t704.check new file mode 100644 index 000000000000..86aa34fce796 --- /dev/null +++ b/tests/run/t704.check @@ -0,0 +1,6 @@ +xxxx should appear twice +zzzz should appear twice +yyyy should appear twice +xxxx should appear twice +zzzz should appear twice +yyyy should appear twice diff --git a/tests/pos/t704.scala b/tests/run/t704.scala similarity index 60% rename from tests/pos/t704.scala rename to tests/run/t704.scala index aedd8c03afdc..6c8d3505ef04 100644 --- a/tests/pos/t704.scala +++ b/tests/run/t704.scala @@ -1,7 +1,12 @@ trait D { private val x = "xxxx should appear twice" - private object xxxx { Console.println(x) } + object xxxx { Console.println(x) } def get_xxxx: AnyRef = xxxx + + private val z = "zzzz should appear twice" + lazy val zzzz = new ZZZZ + class ZZZZ { Console.println(z) } + def get_zzzz: AnyRef = zzzz } trait E extends D { @@ -9,13 +14,15 @@ trait E extends D { val y = "yyyy should appear twice" object yyyy { val x1 = get_xxxx + val z1 = get_zzzz Console.println(y) } yyyy } } class C extends E {} -object Go extends D { +object Test extends D { + object E def main(args : Array[String]): Unit = { new C().f() new C().f() From 2e46d557a018d9e626c794f4248ebd9a369b4a15 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 18 Oct 2019 13:16:42 +0200 Subject: [PATCH 2/3] Put back private modifier in test --- tests/run/t704.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/run/t704.scala b/tests/run/t704.scala index 6c8d3505ef04..cab6f4ca8b42 100644 --- a/tests/run/t704.scala +++ b/tests/run/t704.scala @@ -1,10 +1,10 @@ trait D { private val x = "xxxx should appear twice" - object xxxx { Console.println(x) } + private object xxxx { Console.println(x) } def get_xxxx: AnyRef = xxxx private val z = "zzzz should appear twice" - lazy val zzzz = new ZZZZ + private lazy val zzzz = new ZZZZ class ZZZZ { Console.println(z) } def get_zzzz: AnyRef = zzzz } From af1bde2270f4a1d23ba55cac200d1763a0cab216 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 18 Oct 2019 16:07:14 +0200 Subject: [PATCH 3/3] Fix handling of private trait objects --- .../src/dotty/tools/dotc/transform/LazyVals.scala | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala index d7dcd6f3db55..860afd11d3cc 100644 --- a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala @@ -9,7 +9,7 @@ import dotty.tools.dotc.core.Contexts.Context import dotty.tools.dotc.core.Decorators._ import dotty.tools.dotc.core.DenotTransformers.IdentityDenotTransformer import dotty.tools.dotc.core.Flags._ -import dotty.tools.dotc.core.NameKinds.{LazyBitMapName, LazyLocalInitName, LazyLocalName} +import dotty.tools.dotc.core.NameKinds.{LazyBitMapName, LazyLocalInitName, LazyLocalName, ExpandedName} import dotty.tools.dotc.core.StdNames.nme import dotty.tools.dotc.core.Symbols._ import dotty.tools.dotc.core.Types._ @@ -73,11 +73,18 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer { else { val isField = sym.owner.isClass if (isField) - if sym.isAllOf(SyntheticModule) && sym.allOverriddenSymbols.isEmpty then + if sym.isAllOf(SyntheticModule) + && sym.allOverriddenSymbols.isEmpty + && !sym.name.is(ExpandedName) then // I am not sure what the conditions for this optimization should be. // It was applied for all synthetic objects, but this is clearly false, as t704 demonstrates. // It seems we have to at least exclude synthetic objects that derive from mixins. // This is done by demanding that the object does not override anything. + // Figuring out whether a symbol implements a trait module is not so simple. + // For non-private trait members we can check whether there is an overridden symbol. + // For private trait members this does not work, since `ensureNotPrivate` in phase Mixins + // does change the name but does not update the owner's scope, so `allOverriddenSymbols` does + // not work in that case. However, we can check whether the name is an ExpandedName instead. transformSyntheticModule(tree) else if (sym.isThreadUnsafe || ctx.settings.scalajs.value) if (sym.is(Module) && !ctx.settings.scalajs.value) {