From c16ead8d78936cab3dada7aa5e70eeacc5f576d1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 11 Feb 2016 09:15:32 +0100 Subject: [PATCH 1/5] Abstract out lazy local names somewhat Add operations to NameOps to detect and produce names for lazy locals. @darkdimius Maybe there is already another way to do this? I could not find it. --- src/dotty/tools/dotc/core/NameOps.scala | 7 +++++++ src/dotty/tools/dotc/transform/LazyVals.scala | 12 ++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/dotty/tools/dotc/core/NameOps.scala b/src/dotty/tools/dotc/core/NameOps.scala index 6c1930c9f511..81240a9fc603 100644 --- a/src/dotty/tools/dotc/core/NameOps.scala +++ b/src/dotty/tools/dotc/core/NameOps.scala @@ -82,6 +82,7 @@ object NameOps { def isModuleVarName(name: Name): Boolean = name.stripAnonNumberSuffix endsWith MODULE_VAR_SUFFIX def isSelectorName = name.startsWith(" ") && name.tail.forall(_.isDigit) + def isLazyLocal = name.endsWith(nme.LAZY_LOCAL) /** Is name a variable name? */ def isVariableName: Boolean = name.length > 0 && { @@ -423,5 +424,11 @@ object NameOps { case NO_NAME => primitivePostfixMethodName case name => name } + + def lazyLocalName = name ++ nme.LAZY_LOCAL + def nonLazyName = { + assert(name.isLazyLocal) + name.dropRight(nme.LAZY_LOCAL.length) + } } } diff --git a/src/dotty/tools/dotc/transform/LazyVals.scala b/src/dotty/tools/dotc/transform/LazyVals.scala index 1efc0826b096..4a8ff83caa4c 100644 --- a/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/src/dotty/tools/dotc/transform/LazyVals.scala @@ -63,8 +63,8 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer { if (isField) { if (sym.isVolatile || - (sym.is(Flags.Module) && !sym.is(Flags.Synthetic))) - // module class is user-defined. + (sym.is(Flags.Module) && !sym.is(Flags.Synthetic))) + // module class is user-defined. // Should be threadsafe, to mimic safety guaranteed by global object transformMemberDefVolatile(tree) else if (sym.is(Flags.Module)) { // synthetic module @@ -101,7 +101,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer { */ def transformSyntheticModule(tree: ValOrDefDef)(implicit ctx: Context) = { val sym = tree.symbol - val holderSymbol = ctx.newSymbol(sym.owner, sym.asTerm.name ++ nme.LAZY_LOCAL, + val holderSymbol = ctx.newSymbol(sym.owner, sym.asTerm.name.lazyLocalName, Flags.Synthetic, sym.info.widen.resultType).enteredAfter(this) val field = ValDef(holderSymbol, tree.rhs.changeOwnerAfter(sym, holderSymbol, this)) val getter = DefDef(sym.asTerm, ref(holderSymbol)) @@ -114,7 +114,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer { */ def transformLocalDef(x: ValOrDefDef)(implicit ctx: Context) = { val valueInitter = x.rhs - val holderName = ctx.freshName(x.name ++ StdNames.nme.LAZY_LOCAL).toTermName + val holderName = ctx.freshName(x.name.asTermName.lazyLocalName).toTermName val initName = ctx.freshName(x.name ++ StdNames.nme.LAZY_LOCAL_INIT).toTermName val tpe = x.tpe.widen.resultType.widen @@ -206,7 +206,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer { val claz = x.symbol.owner.asClass val tpe = x.tpe.widen.resultType.widen assert(!(x.mods is Flags.Mutable)) - val containerName = ctx.freshName(x.name ++ StdNames.nme.LAZY_LOCAL).toTermName + val containerName = ctx.freshName(x.name.asTermName.lazyLocalName).toTermName val containerSymbol = ctx.newSymbol(claz, containerName, x.symbol.flags &~ containerFlagsMask | containerFlags | Flags.Private, tpe, coord = x.symbol.coord @@ -367,7 +367,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer { appendOffsetDefs += (companion.moduleClass -> new OffsetInfo(List(offsetTree), ord)) } - val containerName = ctx.freshName(x.name ++ StdNames.nme.LAZY_LOCAL).toTermName + val containerName = ctx.freshName(x.name.asTermName.lazyLocalName).toTermName val containerSymbol = ctx.newSymbol(claz, containerName, (x.mods &~ containerFlagsMask | containerFlags).flags, tpe, coord = x.symbol.coord).enteredAfter(this) val containerTree = ValDef(containerSymbol, defaultValue(tpe)) From df7d8914aa139e8961db6ce0651e09b118d8bab2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 11 Feb 2016 09:17:07 +0100 Subject: [PATCH 2/5] New phase to drop empty companion objects --- src/dotty/tools/dotc/Compiler.scala | 1 + .../dotc/transform/DropEmptyCompanions.scala | 79 +++++++++++++++++++ src/dotty/tools/dotc/transform/Flatten.scala | 1 + 3 files changed, 81 insertions(+) create mode 100644 src/dotty/tools/dotc/transform/DropEmptyCompanions.scala diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index 19965786460d..987c533e71ae 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -77,6 +77,7 @@ class Compiler { new GetClass), // getClass transformation should be applied to specialized methods List(new LambdaLift, // in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here new ElimStaticThis, + new DropEmptyCompanions, new Flatten, new RestoreScopes), List(/*new PrivateToStatic,*/ diff --git a/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala b/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala new file mode 100644 index 000000000000..05296d93e58c --- /dev/null +++ b/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala @@ -0,0 +1,79 @@ +package dotty.tools.dotc +package transform + +import core._ +import DenotTransformers.SymTransformer +import Phases.Phase +import Contexts.Context +import Flags._ +import Symbols._ +import SymDenotations.SymDenotation +import ast.Trees._ +import collection.mutable +import Decorators._ +import NameOps._ +import TreeTransforms.{TreeTransform, MiniPhase} +import dotty.tools.dotc.transform.TreeTransforms.TransformerInfo + +/** Remove companion objects that are empty */ +class DropEmptyCompanions extends MiniPhase { thisTransform => + import ast.tpd._ + override def phaseName = "dropEmpty" + val treeTransform = new Transform(Set()) + + class Transform(dropped: Set[Symbol]) extends TreeTransform { + def phase = thisTransform + + /** Is `tree` an empty companion object? */ + private def isEmptyCompanion(tree: Tree)(implicit ctx: Context) = tree match { + case TypeDef(_, impl: Template) => + tree.symbol.is(Module) && + tree.symbol.companionClass.exists && + impl.body.forall(_.symbol.isPrimaryConstructor) + case _ => + false + } + + /** A transform which has all empty companion objects in `stats` + * recorded in its `dropped` set. + */ + private def localTransform(stats: List[Tree])(implicit ctx: Context) = + new Transform(stats.filter(isEmptyCompanion).map(_.symbol).toSet) + + override def prepareForTemplate(tree: Template)(implicit ctx: Context) = + localTransform(tree.body) + + override def prepareForStats(trees: List[Tree])(implicit ctx: Context) = + if (ctx.owner is Package) localTransform(trees) else this + + /** Symbol is a $lzy field representing a module */ + private def isLazyModuleVar(sym: Symbol)(implicit ctx: Context) = + sym.name.isLazyLocal && + sym.owner.info.decl(sym.name.asTermName.nonLazyName).symbol.is(Module) + + /** Symbol should be dropped together with a dropped companion object. + * Such symbols are: + * - lzy fields pointing to modules, + * - vals and getters representing modules. + */ + private def toDrop(sym: Symbol)(implicit ctx: Context): Boolean = + (sym.is(Module) || isLazyModuleVar(sym)) && + dropped.contains(sym.info.resultType.typeSymbol) + + /** Tree should be dropped because it (is associated with) an empty + * companion object. Such trees are + * - module classes of empty companion objects + * - definitions of lazy module variables or assignments to them. + * - vals and getters for empty companion objects + */ + private def toDrop(stat: Tree)(implicit ctx: Context): Boolean = stat match { + case stat: TypeDef => dropped.contains(stat.symbol) + case stat: ValOrDefDef => toDrop(stat.symbol) + case stat: Assign => toDrop(stat.lhs.symbol) + case _ => false + } + + override def transformStats(stats: List[Tree])(implicit ctx: Context, info: TransformerInfo) = + stats.filterNot(toDrop) + } +} diff --git a/src/dotty/tools/dotc/transform/Flatten.scala b/src/dotty/tools/dotc/transform/Flatten.scala index 9a047ef9589e..f0104e715602 100644 --- a/src/dotty/tools/dotc/transform/Flatten.scala +++ b/src/dotty/tools/dotc/transform/Flatten.scala @@ -11,6 +11,7 @@ import collection.mutable import TreeTransforms.MiniPhaseTransform import dotty.tools.dotc.transform.TreeTransforms.TransformerInfo +/** Lift nested classes to toplevel */ class Flatten extends MiniPhaseTransform with SymTransformer { thisTransform => import ast.tpd._ override def phaseName = "flatten" From 24ccb77061d0b6d3b5fa0464e9afb3e48c67eea9 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 11 Feb 2016 09:19:09 +0100 Subject: [PATCH 3/5] Move test to pending The underlying problem on MacOS/Windows remains: We have a class `B` and an object `b` in the same scope. We used to get a conflict on `B$/b$` because we created an empty companion object for `B`. Now we get a conflict for `B/b`, because the `b` object creates to classes: `b.class` an `b$.class` and `b.class` clashes with `B.class`. --- tests/{ => pending}/run/t920.scala | 5 +++++ 1 file changed, 5 insertions(+) rename tests/{ => pending}/run/t920.scala (96%) diff --git a/tests/run/t920.scala b/tests/pending/run/t920.scala similarity index 96% rename from tests/run/t920.scala rename to tests/pending/run/t920.scala index 6a7f122d55d5..a9874e1a8418 100644 --- a/tests/run/t920.scala +++ b/tests/pending/run/t920.scala @@ -17,4 +17,9 @@ object Test { def main(args : Array[String]) : Unit = { b.initialize; } + class XYZ } + + + + From 1839369a65f4c9d9b076ed657b7c40b5a4418154 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 11 Feb 2016 09:35:09 +0100 Subject: [PATCH 4/5] Only remove synthetic companion objects If the object was explicitly written, it might be referenced, even if it is empty. --- src/dotty/tools/dotc/core/Flags.scala | 1 + src/dotty/tools/dotc/transform/DropEmptyCompanions.scala | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/dotty/tools/dotc/core/Flags.scala b/src/dotty/tools/dotc/core/Flags.scala index 8c9db3a5c965..02b341649adb 100644 --- a/src/dotty/tools/dotc/core/Flags.scala +++ b/src/dotty/tools/dotc/core/Flags.scala @@ -606,6 +606,7 @@ object Flags { final val AbstractFinal = allOf(Abstract, Final) final val AbstractSealed = allOf(Abstract, Sealed) final val SyntheticArtifact = allOf(Synthetic, Artifact) + final val SyntheticModule = allOf(Synthetic, Module) final val SyntheticTermParam = allOf(Synthetic, TermParam) final val SyntheticTypeParam = allOf(Synthetic, TypeParam) final val SyntheticCase = allOf(Synthetic, Case) diff --git a/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala b/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala index 05296d93e58c..71aa92e79fae 100644 --- a/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala +++ b/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala @@ -26,10 +26,12 @@ class DropEmptyCompanions extends MiniPhase { thisTransform => /** Is `tree` an empty companion object? */ private def isEmptyCompanion(tree: Tree)(implicit ctx: Context) = tree match { - case TypeDef(_, impl: Template) => - tree.symbol.is(Module) && + case TypeDef(_, impl: Template) if + tree.symbol.is(SyntheticModule) && tree.symbol.companionClass.exists && - impl.body.forall(_.symbol.isPrimaryConstructor) + impl.body.forall(_.symbol.isPrimaryConstructor) => + //println(i"removing ${tree.symbol}") + true case _ => false } From d3c331782d19994997d920807b16f2edb26dce0d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 11 Feb 2016 17:26:54 +0100 Subject: [PATCH 5/5] Fix problems in DropEmptyCompanions --- src/dotty/tools/dotc/Compiler.scala | 7 +- .../dotc/transform/DropEmptyCompanions.scala | 77 +++++++++++-------- .../tools/dotc/transform/RestoreScopes.scala | 67 +++++++++------- 3 files changed, 89 insertions(+), 62 deletions(-) diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index 987c533e71ae..99b7bd880fff 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -74,14 +74,13 @@ class Compiler { new CapturedVars, // capturedVars has a transformUnit: no phases should introduce local mutable vars here new Constructors, // constructors changes decls in transformTemplate, no InfoTransformers should be added after it new FunctionalInterfaces, - new GetClass), // getClass transformation should be applied to specialized methods + new GetClass), // getClass transformation should be applied to specialized methods List(new LambdaLift, // in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here new ElimStaticThis, - new DropEmptyCompanions, new Flatten, + new DropEmptyCompanions, new RestoreScopes), - List(/*new PrivateToStatic,*/ - new ExpandPrivate, + List(new ExpandPrivate, new CollectEntryPoints, new LabelDefs), List(new GenBCode) diff --git a/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala b/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala index 71aa92e79fae..550e3348f674 100644 --- a/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala +++ b/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala @@ -12,55 +12,55 @@ import ast.Trees._ import collection.mutable import Decorators._ import NameOps._ -import TreeTransforms.{TreeTransform, MiniPhase} +import TreeTransforms.MiniPhaseTransform import dotty.tools.dotc.transform.TreeTransforms.TransformerInfo -/** Remove companion objects that are empty */ -class DropEmptyCompanions extends MiniPhase { thisTransform => +/** Remove companion objects that are empty + * Lots of constraints here: + * 1. It's impractical to place DropEmptyCompanions before lambda lift because dropped + * modules can be anywhere and have hard to trace references. + * 2. DropEmptyCompanions cannot be interleaved with LambdaLift or Flatten because + * they put things in liftedDefs sets which cause them to surface later. So + * removed modules resurface. + * 3. DropEmptyCompanions has to be before RestoreScopes. + * The solution to the constraints is to put DropEmptyCompanions between Flatten + * and RestoreScopes and to only start working once we are back on PackageDef + * level, so we know that all objects moved by LambdaLift and Flatten have arrived + * at their destination. + */ +class DropEmptyCompanions extends MiniPhaseTransform { thisTransform => import ast.tpd._ override def phaseName = "dropEmpty" - val treeTransform = new Transform(Set()) + override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Flatten]) - class Transform(dropped: Set[Symbol]) extends TreeTransform { - def phase = thisTransform + override def transformPackageDef(pdef: PackageDef)(implicit ctx: Context, info: TransformerInfo) = { /** Is `tree` an empty companion object? */ - private def isEmptyCompanion(tree: Tree)(implicit ctx: Context) = tree match { - case TypeDef(_, impl: Template) if - tree.symbol.is(SyntheticModule) && + def isEmptyCompanion(tree: Tree) = tree match { + case TypeDef(_, impl: Template) if tree.symbol.is(SyntheticModule) && tree.symbol.companionClass.exists && impl.body.forall(_.symbol.isPrimaryConstructor) => - //println(i"removing ${tree.symbol}") + println(i"removing ${tree.symbol}") true case _ => false } - /** A transform which has all empty companion objects in `stats` - * recorded in its `dropped` set. - */ - private def localTransform(stats: List[Tree])(implicit ctx: Context) = - new Transform(stats.filter(isEmptyCompanion).map(_.symbol).toSet) - - override def prepareForTemplate(tree: Template)(implicit ctx: Context) = - localTransform(tree.body) - - override def prepareForStats(trees: List[Tree])(implicit ctx: Context) = - if (ctx.owner is Package) localTransform(trees) else this + val dropped = pdef.stats.filter(isEmptyCompanion).map(_.symbol).toSet /** Symbol is a $lzy field representing a module */ - private def isLazyModuleVar(sym: Symbol)(implicit ctx: Context) = + def isLazyModuleVar(sym: Symbol) = sym.name.isLazyLocal && - sym.owner.info.decl(sym.name.asTermName.nonLazyName).symbol.is(Module) + sym.owner.info.decl(sym.name.asTermName.nonLazyName).symbol.is(Module) /** Symbol should be dropped together with a dropped companion object. * Such symbols are: * - lzy fields pointing to modules, * - vals and getters representing modules. */ - private def toDrop(sym: Symbol)(implicit ctx: Context): Boolean = + def symIsDropped(sym: Symbol): Boolean = (sym.is(Module) || isLazyModuleVar(sym)) && - dropped.contains(sym.info.resultType.typeSymbol) + dropped.contains(sym.info.resultType.typeSymbol) /** Tree should be dropped because it (is associated with) an empty * companion object. Such trees are @@ -68,14 +68,31 @@ class DropEmptyCompanions extends MiniPhase { thisTransform => * - definitions of lazy module variables or assignments to them. * - vals and getters for empty companion objects */ - private def toDrop(stat: Tree)(implicit ctx: Context): Boolean = stat match { + def toDrop(stat: Tree): Boolean = stat match { case stat: TypeDef => dropped.contains(stat.symbol) - case stat: ValOrDefDef => toDrop(stat.symbol) - case stat: Assign => toDrop(stat.lhs.symbol) + case stat: ValOrDefDef => symIsDropped(stat.symbol) + case stat: Assign => symIsDropped(stat.lhs.symbol) case _ => false } - override def transformStats(stats: List[Tree])(implicit ctx: Context, info: TransformerInfo) = - stats.filterNot(toDrop) + def prune(tree: Tree): Tree = tree match { + case tree @ TypeDef(name, impl @ Template(constr, _, _, _)) => + cpy.TypeDef(tree)( + rhs = cpy.Template(impl)( + constr = cpy.DefDef(constr)(rhs = pruneLocals(constr.rhs)), + body = pruneStats(impl.body))) + case _ => + tree + } + + def pruneStats(stats: List[Tree]) = + stats.filterConserve(!toDrop(_)).mapConserve(prune) + + def pruneLocals(expr: Tree) = expr match { + case Block(stats, expr) => cpy.Block(expr)(pruneStats(stats), expr) + case _ => expr + } + + cpy.PackageDef(pdef)(pdef.pid, pruneStats(pdef.stats)) } } diff --git a/src/dotty/tools/dotc/transform/RestoreScopes.scala b/src/dotty/tools/dotc/transform/RestoreScopes.scala index fe24186acc90..41da05691d38 100644 --- a/src/dotty/tools/dotc/transform/RestoreScopes.scala +++ b/src/dotty/tools/dotc/transform/RestoreScopes.scala @@ -23,35 +23,46 @@ class RestoreScopes extends MiniPhaseTransform with IdentityDenotTransformer { t import ast.tpd._ override def phaseName = "restoreScopes" - override def transformTypeDef(tree: TypeDef)(implicit ctx: Context, info: TransformerInfo) = { - val TypeDef(_, impl: Template) = tree - // - val restoredDecls = newScope - for (stat <- impl.constr :: impl.body) - if (stat.isInstanceOf[MemberDef] && stat.symbol.exists) - restoredDecls.enter(stat.symbol) + /* Note: We need to wait until we see a package definition because + * DropEmptyConstructors changes template members when analyzing the + * enclosing package definitions. So by the time RestoreScopes gets to + * see a typedef or template, it still might be changed by DropEmptyConstructors. + */ + override def transformPackageDef(pdef: PackageDef)(implicit ctx: Context, info: TransformerInfo) = { + pdef.stats.foreach(restoreScope) + pdef + } + + private def restoreScope(tree: Tree)(implicit ctx: Context, info: TransformerInfo) = tree match { + case TypeDef(_, impl: Template) => + val restoredDecls = newScope + for (stat <- impl.constr :: impl.body) + if (stat.isInstanceOf[MemberDef] && stat.symbol.exists) + restoredDecls.enter(stat.symbol) // Enter class in enclosing package scope, in case it was an inner class before flatten. // For top-level classes this does nothing. - val cls = tree.symbol.asClass - val pkg = cls.owner.asClass - - // Bring back companion links - val companionClass = cls.info.decls.lookup(nme.COMPANION_CLASS_METHOD) - val companionModule = cls.info.decls.lookup(nme.COMPANION_MODULE_METHOD) - - if (companionClass.exists) { - restoredDecls.enter(companionClass) - } - - if (companionModule.exists) { - restoredDecls.enter(companionModule) - } - - pkg.enter(cls) - val cinfo = cls.classInfo - tree.symbol.copySymDenotation( - info = cinfo.derivedClassInfo( // Dotty deviation: Cannot expand cinfo inline without a type error - decls = restoredDecls: Scope)).installAfter(thisTransform) - tree + val cls = tree.symbol.asClass + val pkg = cls.owner.asClass + + // Bring back companion links + val companionClass = cls.info.decls.lookup(nme.COMPANION_CLASS_METHOD) + val companionModule = cls.info.decls.lookup(nme.COMPANION_MODULE_METHOD) + + if (companionClass.exists) { + restoredDecls.enter(companionClass) + } + + if (companionModule.exists) { + restoredDecls.enter(companionModule) + } + + pkg.enter(cls) + val cinfo = cls.classInfo + tree.symbol.copySymDenotation( + info = cinfo.derivedClassInfo( // Dotty deviation: Cannot expand cinfo inline without a type error + decls = restoredDecls: Scope)).installAfter(thisTransform) + tree + case tree => tree } } +