From 86213e3f200dc18b86c14edd1657a74bbd09e95b Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Thu, 26 May 2016 18:42:43 +0200 Subject: Backport from Linker: Fix to lambda lift. Fixes #1280. CollectDependencies had incorrect handling of `Ident`s. If it had ever met an `Ident` to a symbol defined outside of current owner-chain(e.g. Predef.println) it would issue narrowTo(enclosingClass). This is very conservative and leads to memory leaks even in trivial lambdas. My lambda-lift-foo groes stronger :-) --- 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 3ef684e55..2d12a1d3c 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -249,14 +249,21 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform else if (sym is Method) markCalled(sym, enclosure) else if (sym.isTerm) markFree(sym, enclosure) } - if (sym.maybeOwner.isClass) narrowTo(sym.owner.asClass) + def captureImplicitThis(x: Type): Unit = { + x match { + case TermRef(x, _) => captureImplicitThis(x) + case x: ThisType => narrowTo(x.tref.typeSymbol.asClass) + case _ => + } + } + captureImplicitThis(tree.tpe) case tree: Select => if (sym.is(Method) && isLocal(sym)) markCalled(sym, enclosure) case tree: This => narrowTo(tree.symbol.asClass) case tree: DefDef => if (sym.owner.isTerm && !sym.is(Label)) - liftedOwner(sym) = sym.enclosingClass.topLevelClass + liftedOwner(sym) = sym.enclosingPackageClass // 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 -- cgit v1.2.3 From f85663af4badc25cc5645fb811c616055e2702bb Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Thu, 26 May 2016 18:44:22 +0200 Subject: Backport from Linker: ElimStaticThis: allow more calls to static methods Before there was an implicit assumption that static methods are only present on objects. This assumption is invalidated both by linker optimizations and the new fix to lambdalift. --- src/dotty/tools/dotc/transform/ElimStaticThis.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/ElimStaticThis.scala b/src/dotty/tools/dotc/transform/ElimStaticThis.scala index 70a610188..3afcfa685 100644 --- a/src/dotty/tools/dotc/transform/ElimStaticThis.scala +++ b/src/dotty/tools/dotc/transform/ElimStaticThis.scala @@ -27,9 +27,11 @@ class ElimStaticThis extends MiniPhaseTransform { override def transformIdent(tree: tpd.Ident)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { if (ctx.owner.enclosingMethod.is(JavaStatic)) { tree.tpe match { - case TermRef(thiz: ThisType, _) => - assert(thiz.underlying.typeSymbol.is(ModuleClass)) + case TermRef(thiz: ThisType, _) if thiz.underlying.typeSymbol.is(ModuleClass) => ref(thiz.underlying.typeSymbol.sourceModule).select(tree.symbol) + case TermRef(thiz: ThisType, _) => + assert(tree.symbol.is(Flags.JavaStatic)) + tree case _ => tree } } -- cgit v1.2.3 From 4007910927bf494c917ac1ffcbf8b73783541247 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Thu, 26 May 2016 22:47:10 +0200 Subject: Fix deadlock in t5375 and similar tests. See t5375.scala for details. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 5 +++-- tests/run/t5375.scala | 11 +++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 2d12a1d3c..83f272b01 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -369,8 +369,9 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform // though the second condition seems weird, it's not true for symbols which are defined in some // weird combinations of super calls. (encClass, EmptyFlags) - } else - (topClass, JavaStatic) + } else if (encClass.is(ModuleClass, butNot = Package) && encClass.isStatic) // needed to not cause deadlocks in classloader. see t5375.scala + (encClass, EmptyFlags) + else (topClass, JavaStatic) } else (lOwner, EmptyFlags) local.copySymDenotation( diff --git a/tests/run/t5375.scala b/tests/run/t5375.scala index 8c2c06fde..d8413c1a3 100644 --- a/tests/run/t5375.scala +++ b/tests/run/t5375.scala @@ -1,3 +1,14 @@ +/** Hello fellow compiler developer. + if you are wondering why does test suite hang on this test + then it's likely that the lambda inside map has been compiled into static method + unfotrunatelly, as it is executed inside static object initializer, + it is executed inside class-loader, in a synchronized block that is not source defined. + + If the lambda will be static Test$#foo, calling it through a different thread would require grabbing the + lock inside classloader. Unlike if it not static and is called through This(Test).foo, no lock is grabbed. + + @DarkDimius +*/ object Test extends dotty.runtime.LegacyApp { val foos = (1 to 1000).toSeq try -- cgit v1.2.3 From 3b0cb481e379ee07ce33d80a1bee5ff67c32eb40 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Fri, 27 May 2016 13:18:52 +0200 Subject: LambdaLift: do not close over members that are static. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 83f272b01..5fbe0343f 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -251,8 +251,8 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform } def captureImplicitThis(x: Type): Unit = { x match { - case TermRef(x, _) => captureImplicitThis(x) - case x: ThisType => narrowTo(x.tref.typeSymbol.asClass) + case tr@TermRef(x, _) if (!tr.termSymbol.isStatic) => captureImplicitThis(x) + case x: ThisType if (!x.tref.typeSymbol.isStaticOwner) => narrowTo(x.tref.typeSymbol.asClass) case _ => } } -- cgit v1.2.3