From 07ed3f159b79e53874f53a2c5aa2b00aa2ae0ded Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 1 Jun 2015 10:05:56 +0200 Subject: [PATCH 1/8] Revert "Avoid static initialization deadlock in run tests." This reverts commit 8f90105dc4e62e78d53b385df1b2eb29f2855183. --- tests/run/t5375.scala | 13 +++++-------- tests/run/t6052.scala | 10 ++++------ tests/run/t6410.scala | 12 +++++------- tests/run/t6467.scala | 12 +++++------- tests/run/t7498.scala | 10 ++++------ 5 files changed, 23 insertions(+), 34 deletions(-) diff --git a/tests/run/t5375.scala b/tests/run/t5375.scala index 256f5435e0b5..8c2c06fde30a 100644 --- a/tests/run/t5375.scala +++ b/tests/run/t5375.scala @@ -1,11 +1,8 @@ -object Test { +object Test extends dotty.runtime.LegacyApp { val foos = (1 to 1000).toSeq - - def main(args: Array[String]): Unit = { - try - foos.par.map(i => if (i % 37 == 0) sys.error("i div 37") else i) - catch { - case ex: RuntimeException => println("Runtime exception") - } + try + foos.par.map(i => if (i % 37 == 0) sys.error("i div 37") else i) + catch { + case ex: RuntimeException => println("Runtime exception") } } diff --git a/tests/run/t6052.scala b/tests/run/t6052.scala index 15bffc1eb946..e740f00e16d0 100644 --- a/tests/run/t6052.scala +++ b/tests/run/t6052.scala @@ -5,7 +5,7 @@ -object Test { +object Test extends dotty.runtime.LegacyApp { def seqarr(i: Int) = Array[Int]() ++ (0 until i) def pararr(i: Int) = seqarr(i).par @@ -15,9 +15,7 @@ object Test { assert(gseq == gpar, (gseq, gpar)) } - def main(args: Array[String]): Unit = { - for (i <- 0 until 20) check(i, _ > 0) - for (i <- 0 until 20) check(i, _ % 2) - for (i <- 0 until 20) check(i, _ % 4) - } + for (i <- 0 until 20) check(i, _ > 0) + for (i <- 0 until 20) check(i, _ % 2) + for (i <- 0 until 20) check(i, _ % 4) } diff --git a/tests/run/t6410.scala b/tests/run/t6410.scala index 51aaad839e5a..0855ffecdb2e 100644 --- a/tests/run/t6410.scala +++ b/tests/run/t6410.scala @@ -1,11 +1,9 @@ -object Test { - def main(args: Array[String]): Unit = { - val x = collection.parallel.mutable.ParArray.range(1,10) groupBy { _ % 2 } mapValues { _.size } - println(x) - val y = collection.parallel.immutable.ParVector.range(1,10) groupBy { _ % 2 } mapValues { _.size } - println(y) - } +object Test extends dotty.runtime.LegacyApp { + val x = collection.parallel.mutable.ParArray.range(1,10) groupBy { _ % 2 } mapValues { _.size } + println(x) + val y = collection.parallel.immutable.ParVector.range(1,10) groupBy { _ % 2 } mapValues { _.size } + println(y) } diff --git a/tests/run/t6467.scala b/tests/run/t6467.scala index 048ef25e2403..e02fb166993e 100644 --- a/tests/run/t6467.scala +++ b/tests/run/t6467.scala @@ -6,17 +6,15 @@ import collection._ -object Test { +object Test extends dotty.runtime.LegacyApp { def compare(s1: String, s2: String): Unit = { assert(s1 == s2, s1 + "\nvs.\n" + s2) } - def main(args: Array[String]): Unit = { - compare(List(1, 2, 3, 4).aggregate(new java.lang.StringBuffer)(_ append _, _ append _).toString, "1234") - compare(List(1, 2, 3, 4).par.aggregate(new java.lang.StringBuffer)(_ append _, _ append _).toString, "1234") - compare(Seq(0 until 100: _*).aggregate(new java.lang.StringBuffer)(_ append _, _ append _).toString, (0 until 100).mkString) - compare(Seq(0 until 100: _*).par.aggregate(new java.lang.StringBuffer)(_ append _, _ append _).toString, (0 until 100).mkString) - } + compare(List(1, 2, 3, 4).aggregate(new java.lang.StringBuffer)(_ append _, _ append _).toString, "1234") + compare(List(1, 2, 3, 4).par.aggregate(new java.lang.StringBuffer)(_ append _, _ append _).toString, "1234") + compare(Seq(0 until 100: _*).aggregate(new java.lang.StringBuffer)(_ append _, _ append _).toString, (0 until 100).mkString) + compare(Seq(0 until 100: _*).par.aggregate(new java.lang.StringBuffer)(_ append _, _ append _).toString, (0 until 100).mkString) } diff --git a/tests/run/t7498.scala b/tests/run/t7498.scala index a2555c6b11a2..cab598405766 100644 --- a/tests/run/t7498.scala +++ b/tests/run/t7498.scala @@ -5,18 +5,16 @@ -object Test { +object Test extends dotty.runtime.LegacyApp { import scala.collection.concurrent.TrieMap class Collision(val idx: Int) { override def hashCode = idx % 10 } - def main(args: Array[String]): Unit = { - val tm = TrieMap[Collision, Unit]() - for (i <- 0 until 1000) tm(new Collision(i)) = () + val tm = TrieMap[Collision, Unit]() + for (i <- 0 until 1000) tm(new Collision(i)) = () - tm.par.foreach(kv => ()) - } + tm.par.foreach(kv => ()) } From bb7d6ddfeb22f07658466f731038e270e0af99e9 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 1 Jun 2015 10:22:35 +0200 Subject: [PATCH 2/8] Avoid static initialization deadlock in run tests (2). See https://github.com/lampepfl/dotty/pull/624#issuecomment-107064519 for a lengthy explanation. We now solve the problem in LambdaLift. The formerly failing tests are all reverted to theor original, failing, version. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 42c6e85af4d1..81e4d1ff3302 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -210,8 +210,15 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform case tree: This => narrowTo(tree.symbol.asClass) case tree: DefDef => - if (sym.owner.isTerm && !sym.is(Label)) liftedOwner(sym) = sym.topLevelClass.owner - else if (sym.isPrimaryConstructor && sym.owner.owner.isTerm) symSet(called, sym) += sym.owner + if (sym.owner.isTerm && !sym.is(Label)) + liftedOwner(sym) = sym.enclosingClass.topLevelClass + // this will make methods in supercall constructors of top-level classes owned + // by the enclosing package, which means they will be static. + // On the other hand, all other methods will be indirectly owned by their + // top-level class. This avoids possible deadlocks when a static method + // has to access its enclosing object from the outside. + else if (sym.isPrimaryConstructor && sym.owner.owner.isTerm) + symSet(called, sym) += sym.owner case tree: TypeDef => if (sym.owner.isTerm) liftedOwner(sym) = sym.topLevelClass.owner case tree: Template => From f0787353e407a01a4b0a5f814c3d0feeff7cd87e Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Tue, 2 Jun 2015 11:01:50 +0200 Subject: [PATCH 3/8] Revert "Update t6260-delambdafy.check to account for change in lambda generation" This reverts commit cafd71af4902c76561f27a479c14e53729600bb9. For the future refference: tests and checkfiles should be modified only after carefull thought. Otherwise our tests will stop indicating the correct behaviour. --- tests/run/t6260-delambdafy.check | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/run/t6260-delambdafy.check b/tests/run/t6260-delambdafy.check index eb7212adc211..b3ec1b3cc667 100644 --- a/tests/run/t6260-delambdafy.check +++ b/tests/run/t6260-delambdafy.check @@ -1,3 +1,4 @@ f(C@2e) apply +get$Lambda From 4afa3ffe68f450c1f47018892489a0f2a9b4b9b5 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Tue, 2 Jun 2015 11:06:07 +0200 Subject: [PATCH 4/8] Revert "Fix ElimStaticThis#transformIdent" This reverts commit 3d240ad40ccfb570174ec9758bfe68ba4e91eefb. This commit got in without succeding the review. It broke what already was working(inner static objects), and made impossible moving static methods from companion object to companion class. Additionally, commenting or removing assertions is not the way to go, and should not pass review. See discussion here: https://github.com/dotty-staging/dotty/commit/3d240ad40ccfb570174ec9758bfe68ba4e91eefb --- src/dotty/tools/dotc/Compiler.scala | 2 +- src/dotty/tools/dotc/transform/ElimStaticThis.scala | 11 +++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index 41e77c61f95a..d7ef441441a9 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -67,8 +67,8 @@ class Compiler { new Constructors, new FunctionalInterfaces), List(new LambdaLift, // in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here - new Flatten, new ElimStaticThis, + new Flatten, new RestoreScopes), List(/*new PrivateToStatic,*/ new ExpandPrivate, diff --git a/src/dotty/tools/dotc/transform/ElimStaticThis.scala b/src/dotty/tools/dotc/transform/ElimStaticThis.scala index c609904772e5..7df29b0b079a 100644 --- a/src/dotty/tools/dotc/transform/ElimStaticThis.scala +++ b/src/dotty/tools/dotc/transform/ElimStaticThis.scala @@ -9,7 +9,6 @@ import dotty.tools.dotc.core.StdNames._ import dotty.tools.dotc.core.SymDenotations.SymDenotation import TreeTransforms.{MiniPhaseTransform, TransformerInfo} import dotty.tools.dotc.core.Types.{ThisType, TermRef} -import Phases.Phase /** Replace This references to module classes in static methods by global identifiers to the * corresponding modules. @@ -18,8 +17,6 @@ class ElimStaticThis extends MiniPhaseTransform { import ast.tpd._ def phaseName: String = "elimStaticThis" - override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Flatten]) - override def transformThis(tree: This)(implicit ctx: Context, info: TransformerInfo): Tree = if (!tree.symbol.is(Package) && ctx.owner.enclosingMethod.is(JavaStatic)) { assert(tree.symbol.is(ModuleClass)) @@ -28,12 +25,10 @@ class ElimStaticThis extends MiniPhaseTransform { else tree override def transformIdent(tree: tpd.Ident)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { - val meth = ctx.owner.enclosingMethod - // We cannot use meth.enclosingClass because it skips other static classes, - // so instead we require this phase to run after Flatten and use meth.owner - if (meth.is(JavaStatic) && meth.owner.is(ModuleClass)) { + if (ctx.owner.enclosingMethod.is(JavaStatic)) { tree.tpe match { - case TermRef(thiz: ThisType, _) if (thiz.underlying.typeSymbol == meth.owner) => + case TermRef(thiz: ThisType, _) => + assert(thiz.underlying.typeSymbol.is(ModuleClass)) ref(thiz.underlying.typeSymbol.sourceModule).select(tree.symbol) case _ => tree } From 367c9d36622dd0a37304efd042374c20944d75bd Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Tue, 2 Jun 2015 14:49:09 +0200 Subject: [PATCH 5/8] Move test from lambdaLift.scala to a separate file in pending. --- tests/pending/pos/lambdalift-1.scala | 20 ++++++++++++++++++++ tests/pos/lambdalift.scala | 21 --------------------- 2 files changed, 20 insertions(+), 21 deletions(-) create mode 100644 tests/pending/pos/lambdalift-1.scala diff --git a/tests/pending/pos/lambdalift-1.scala b/tests/pending/pos/lambdalift-1.scala new file mode 100644 index 000000000000..269f839d6bf0 --- /dev/null +++ b/tests/pending/pos/lambdalift-1.scala @@ -0,0 +1,20 @@ +class Super(x: Int) + +class Sub extends Super({ + def foo3(x: Int) = { + + class C { + def this(name: String) = this() + + def bam(y: Int): String => Int = { + def baz = x + y + z => baz * z.length + } + } + + val fun = new C("dummy").bam(1) + fun("abc") + + } + foo3(22) +}) diff --git a/tests/pos/lambdalift.scala b/tests/pos/lambdalift.scala index febae6828bc6..33cc2c069e9b 100644 --- a/tests/pos/lambdalift.scala +++ b/tests/pos/lambdalift.scala @@ -23,24 +23,3 @@ object test { } } - -class Super(x: Int) - -class Sub extends Super({ - def foo3(x: Int) = { - - class C { - def this(name: String) = this() - - def bam(y: Int): String => Int = { - def baz = x + y - z => baz * z.length - } - } - - val fun = new C("dummy").bam(1) - fun("abc") - - } - foo3(22) -}) From 5cb4ecb19cb551609ebee97aa1b199de8468a98a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 3 Jun 2015 17:56:11 +0200 Subject: [PATCH 6/8] Narrow liftedOwner also for static objects Lambdas should stay inside static objects if they reference to them with a This or Ident. Fixes test case `llist.scala` but demonstrates another problem. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 81e4d1ff3302..306a03642a8d 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -189,10 +189,9 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform val sym = tree.symbol def narrowTo(thisClass: ClassSymbol) = { val enclClass = enclosure.enclosingClass - if (!thisClass.isStaticOwner) - narrowLiftedOwner(enclosure, - if (enclClass.isContainedIn(thisClass)) thisClass - else enclClass) // unknown this reference, play it safe and assume the narrowest possible owner + narrowLiftedOwner(enclosure, + if (enclClass.isContainedIn(thisClass)) thisClass + else enclClass) // unknown this reference, play it safe and assume the narrowest possible owner } tree match { case tree: Ident => From 6c8a2425148753d040ab2dbde6ac57349893b736 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 3 Jun 2015 17:59:16 +0200 Subject: [PATCH 7/8] Compute outer.path in lambdaLift at correct phase. LambdaLift needs to compute outer.path at the phase in which the results are constructed, i.e. phase lambdaLift.next. Or else we get an error in outer.path for lost fo files, including pos/Fileish.scala as a minimized test case. Previously outer as computed at phase lambdaLift. The reason for this is that lambdaLift name mangles inner classes, which causes outer acessors to be not found. We now correct for the problem in outer.path itself, by calling outerAccessor only at a safe phase. --- src/dotty/tools/dotc/core/Contexts.scala | 3 +++ src/dotty/tools/dotc/transform/ExplicitOuter.scala | 5 +++-- src/dotty/tools/dotc/transform/LambdaLift.scala | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/dotty/tools/dotc/core/Contexts.scala b/src/dotty/tools/dotc/core/Contexts.scala index c1b2e20eb687..b00896117750 100644 --- a/src/dotty/tools/dotc/core/Contexts.scala +++ b/src/dotty/tools/dotc/core/Contexts.scala @@ -231,6 +231,9 @@ object Contexts { final def withPhase(phase: Phase): Context = withPhase(phase.id) + final def withPhaseNoLater(phase: Phase) = + if (ctx.phase.id > phase.id) withPhase(phase) else ctx + /** If -Ydebug is on, the top of the stack trace where this context * was created, otherwise `null`. */ diff --git a/src/dotty/tools/dotc/transform/ExplicitOuter.scala b/src/dotty/tools/dotc/transform/ExplicitOuter.scala index 70a4f299a37d..0fa429d6ef19 100644 --- a/src/dotty/tools/dotc/transform/ExplicitOuter.scala +++ b/src/dotty/tools/dotc/transform/ExplicitOuter.scala @@ -299,9 +299,10 @@ object ExplicitOuter { def path(toCls: Symbol): Tree = try { def loop(tree: Tree): Tree = { val treeCls = tree.tpe.widen.classSymbol - ctx.log(i"outer to $toCls of $tree: ${tree.tpe}, looking for ${outerAccName(treeCls.asClass)}") + val outerAccessorCtx = ctx.withPhaseNoLater(ctx.lambdaLiftPhase) // lambdalift mangles local class names, which means we cannot reliably find outer acessors anymore + ctx.log(i"outer to $toCls of $tree: ${tree.tpe}, looking for ${outerAccName(treeCls.asClass)(outerAccessorCtx)} in $treeCls") if (treeCls == toCls) tree - else loop(tree select outerAccessor(treeCls.asClass)) + else loop(tree select outerAccessor(treeCls.asClass)(outerAccessorCtx)) } ctx.log(i"computing outerpath to $toCls from ${ctx.outersIterator.map(_.owner).toList}") loop(This(ctx.owner.enclosingClass.asClass)) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 306a03642a8d..bffc7458e456 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -94,7 +94,7 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform * than the previous value of `liftedowner(sym)`. */ def narrowLiftedOwner(sym: Symbol, owner: Symbol)(implicit ctx: Context) = { - if (sym.owner.isTerm && + if (sym.maybeOwner.isTerm && owner.isProperlyContainedIn(liftedOwner(sym)) && owner != sym) { ctx.log(i"narrow lifted $sym to $owner") @@ -366,7 +366,7 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform val clazz = sym.enclosingClass val qual = if (clazz.isStaticOwner) singleton(clazz.thisType) - else outer(ctx.withPhase(thisTransform)).path(clazz) + else outer.path(clazz) transformFollowingDeep(qual.select(sym)) } From bcf70e27e0eef1b55c2e4afd8f680c0351810073 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 3 Jun 2015 17:59:29 +0200 Subject: [PATCH 8/8] Test case for problems with lambda lifting. --- tests/pos/llift.scala | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/pos/llift.scala diff --git a/tests/pos/llift.scala b/tests/pos/llift.scala new file mode 100644 index 000000000000..51631ac970fd --- /dev/null +++ b/tests/pos/llift.scala @@ -0,0 +1,9 @@ +class A { + class B { + def outer(): Unit = { + def inner(): Int = 2 + + val fi: Function0[Int] = () => inner() + } + } +}