From b0856feefb90d1c0647bb3725ed9797fc76e0603 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Fri, 8 May 2015 11:13:14 +0200 Subject: [PATCH 1/4] Fix #545: no need to make members of static classes static. Otherwise we will need to rewrite references to `This` of class be references on ModuleVal. This is less efficient(instead of calling method statically known to be final, you have virtual call) and less jvm-friendly, as needs additional instructions to get to ModuleVal. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 7be0a12abeae..bac088928bc9 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -288,7 +288,13 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform private def liftLocals()(implicit ctx: Context): Unit = { for ((local, lOwner) <- liftedOwner) { val (newOwner, maybeStatic) = - if (lOwner is Package) (local.topLevelClass, JavaStatic) + if (lOwner is Package) { + println(s"lifting $local encl class ${local.enclosingClass}") + if (local.enclosingClass.isStatic) // member of a static object + (local.enclosingClass, EmptyFlags) + else + (local.topLevelClass, JavaStatic) + } else (lOwner, EmptyFlags) local.copySymDenotation( owner = newOwner, From d109796791b1c2e4b96c172531f248902afb2755 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Fri, 8 May 2015 11:15:46 +0200 Subject: [PATCH 2/4] Update comment on liftedOwner. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index bac088928bc9..060fb563faa9 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -66,7 +66,7 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform /** A map from local methods and classes to the owners to which they will be lifted as members. * For methods and classes that do not have any dependencies this will be the enclosing package. * symbols with packages as lifted owners will subsequently represented as static - * members of their toplevel class. + * members of their toplevel class, unless their enclosing class was already static. */ private val liftedOwner = new HashMap[Symbol, Symbol] @@ -289,7 +289,6 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform for ((local, lOwner) <- liftedOwner) { val (newOwner, maybeStatic) = if (lOwner is Package) { - println(s"lifting $local encl class ${local.enclosingClass}") if (local.enclosingClass.isStatic) // member of a static object (local.enclosingClass, EmptyFlags) else From 32a70042fc8a0338e093dba0424c98d25fb3a754 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Fri, 8 May 2015 12:41:14 +0200 Subject: [PATCH 3/4] Guard agains lifting symbols defined in super-calls to non-static members of package. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 060fb563faa9..c22167b99be8 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -289,9 +289,12 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform for ((local, lOwner) <- liftedOwner) { val (newOwner, maybeStatic) = if (lOwner is Package) { - if (local.enclosingClass.isStatic) // member of a static object + // member of a static object + if (local.enclosingClass.isStatic && local.enclosingClass.isProperlyContainedIn(local.topLevelClass)) { + // though the second condition seems weird, it's not true for symbols which are defined in some + // weird combinations of super calls. (local.enclosingClass, EmptyFlags) - else + } else (local.topLevelClass, JavaStatic) } else (lOwner, EmptyFlags) From 929e8893522d1cbfdfbb7d2e7c6531d3435420ec Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Wed, 13 May 2015 14:11:02 +0200 Subject: [PATCH 4/4] Compute enclosingClass and topLevelClass once in LambdaLift.liftLocals --- src/dotty/tools/dotc/transform/LambdaLift.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index c22167b99be8..42c6e85af4d1 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -289,13 +289,15 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform for ((local, lOwner) <- liftedOwner) { val (newOwner, maybeStatic) = if (lOwner is Package) { + val encClass = local.enclosingClass + val topClass = local.topLevelClass // member of a static object - if (local.enclosingClass.isStatic && local.enclosingClass.isProperlyContainedIn(local.topLevelClass)) { + if (encClass.isStatic && encClass.isProperlyContainedIn(topClass)) { // though the second condition seems weird, it's not true for symbols which are defined in some // weird combinations of super calls. - (local.enclosingClass, EmptyFlags) + (encClass, EmptyFlags) } else - (local.topLevelClass, JavaStatic) + (topClass, JavaStatic) } else (lOwner, EmptyFlags) local.copySymDenotation(