diff options
author | Dmitry Petrashko <dark@d-d.me> | 2015-09-14 11:43:17 +0200 |
---|---|---|
committer | Dmitry Petrashko <dark@d-d.me> | 2015-09-14 11:43:17 +0200 |
commit | 4f2e912d38bef3c3ccb799dd2d2ede7e3f73d00e (patch) | |
tree | 958c5caf3c367b35c0e51f4e96951d60aa4622b6 | |
parent | 7f01a79ae9e368a4b6dfe70bff0c8c9ad6744ac6 (diff) | |
parent | 863d72dea0ada792ec3813fce588a996c532d295 (diff) | |
download | dotty-4f2e912d38bef3c3ccb799dd2d2ede7e3f73d00e.tar.gz dotty-4f2e912d38bef3c3ccb799dd2d2ede7e3f73d00e.tar.bz2 dotty-4f2e912d38bef3c3ccb799dd2d2ede7e3f73d00e.zip |
Merge pull request #774 from dotty-staging/fix-constructors
Constructors: fixes to maintain needed private fields.
-rw-r--r-- | src/dotty/tools/dotc/Compiler.scala | 2 | ||||
-rw-r--r-- | src/dotty/tools/dotc/transform/Constructors.scala | 106 | ||||
-rw-r--r-- | src/dotty/tools/dotc/transform/Memoize.scala | 14 |
3 files changed, 72 insertions, 50 deletions
diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index f753b7614..e4b328a82 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -71,7 +71,7 @@ class Compiler { new LinkScala2ImplClasses, new NonLocalReturns, new CapturedVars, // capturedVars has a transformUnit: no phases should introduce local mutable vars here - new Constructors, + 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 List(new LambdaLift, // in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here diff --git a/src/dotty/tools/dotc/transform/Constructors.scala b/src/dotty/tools/dotc/transform/Constructors.scala index ad3422232..265ad3217 100644 --- a/src/dotty/tools/dotc/transform/Constructors.scala +++ b/src/dotty/tools/dotc/transform/Constructors.scala @@ -33,6 +33,58 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Memoize]) + // Collect all private parameter accessors and value definitions that need + // to be retained. There are several reasons why a parameter accessor or + // definition might need to be retained: + // 1. It is accessed after the constructor has finished + // 2. It is accessed before it is defined + // 3. It is accessed on an object other than `this` + // 4. It is a mutable parameter accessor + // 5. It is has a wildcard initializer `_` + private val retainedPrivateVals = mutable.Set[Symbol]() + private val seenPrivateVals = mutable.Set[Symbol]() + + private def markUsedPrivateSymbols(tree: RefTree)(implicit ctx: Context): Unit = { + + val sym = tree.symbol + def retain() = + retainedPrivateVals.add(sym) + + if (sym.exists && sym.owner.isClass && mightBeDropped(sym)) { + val owner = sym.owner.asClass + + tree match { + case Ident(_) | Select(This(_), _) => + def inConstructor = { + val method = ctx.owner.enclosingMethod + method.isPrimaryConstructor && ctx.owner.enclosingClass == owner + } + if (inConstructor && (sym.is(ParamAccessor) || seenPrivateVals.contains(sym))) { + // used inside constructor, accessed on this, + // could use constructor argument instead, no need to retain field + } + else retain() + case _ => retain() + } + } + } + + override def transformIdent(tree: tpd.Ident)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { + markUsedPrivateSymbols(tree) + tree + } + + override def transformSelect(tree: tpd.Select)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { + markUsedPrivateSymbols(tree) + tree + } + + override def transformValDef(tree: tpd.ValDef)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { + if (mightBeDropped(tree.symbol)) + (if (isWildcardStarArg(tree.rhs)) retainedPrivateVals else seenPrivateVals) += tree.symbol + tree + } + /** All initializers for non-lazy fields should be moved into constructor. * All non-abstract methods should be implemented (this is assured for constructors * in this phase and for other methods in memoize). @@ -75,6 +127,7 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor override def transformTemplate(tree: Template)(implicit ctx: Context, info: TransformerInfo): Tree = { val cls = ctx.owner.asClass + val constr @ DefDef(nme.CONSTRUCTOR, Nil, vparams :: Nil, _, EmptyTree) = tree.constr // Produce aligned accessors and constructor parameters. We have to adjust @@ -113,46 +166,9 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor } } - // Collect all private parameter accessors and value definitions that need - // to be retained. There are several reasons why a parameter accessor or - // definition might need to be retained: - // 1. It is accessed after the constructor has finished - // 2. It is accessed before it is defined - // 3. It is accessed on an object other than `this` - // 4. It is a mutable parameter accessor - // 5. It is has a wildcard initializer `_` - object usage extends TreeTraverser { - private var inConstr: Boolean = true - private val seen = mutable.Set[Symbol](accessors: _*) - val retained = mutable.Set[Symbol]() - def dropped: collection.Set[Symbol] = seen -- retained - override def traverse(tree: Tree)(implicit ctx: Context) = { - val sym = tree.symbol - tree match { - case Ident(_) | Select(This(_), _) if inConstr && seen(tree.symbol) => - // could refer to definition in constructors, so no retention necessary - case tree: RefTree => - if (mightBeDropped(sym)) retained += sym - case _ => - } - if (!noDirectRefsFrom(tree)) traverseChildren(tree) - } - def collect(stats: List[Tree]): Unit = stats foreach { - case stat: ValDef if !stat.symbol.is(Lazy) => - traverse(stat) - if (mightBeDropped(stat.symbol)) - (if (isWildcardStarArg(stat.rhs)) retained else seen) += stat.symbol - case stat: DefTree => - inConstr = false - traverse(stat) - inConstr = true - case stat => - traverse(stat) - } + def isRetained(acc: Symbol) = { + !mightBeDropped(acc) || retainedPrivateVals(acc) } - usage.collect(tree.body) - - def isRetained(acc: Symbol) = !mightBeDropped(acc) || usage.retained(acc) val constrStats, clsStats = new mutable.ListBuffer[Tree] @@ -170,6 +186,8 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor } } + val dropped = mutable.Set[Symbol]() + // Split class body into statements that go into constructor and // definitions that are kept as members of the class. def splitStats(stats: List[Tree]): Unit = stats match { @@ -183,6 +201,7 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor clsStats += cpy.ValDef(stat)(rhs = EmptyTree) } else if (!stat.rhs.isEmpty) { + dropped += sym sym.copySymDenotation( initFlags = sym.flags &~ Private, owner = constr.symbol).installAfter(thisTransform) @@ -203,8 +222,10 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor // The initializers for the retained accessors */ val copyParams = accessors flatMap { acc => - if (!isRetained(acc)) Nil - else { + if (!isRetained(acc)) { + dropped += acc + Nil + } else { val target = if (acc.is(Method)) acc.field else acc if (!target.exists) Nil // this case arises when the parameter accessor is an alias else { @@ -224,12 +245,13 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor } // Drop accessors that are not retained from class scope - val dropped = usage.dropped if (dropped.nonEmpty) { val clsInfo = cls.classInfo // TODO investigate: expand clsInfo to cls.info => dotty type error cls.copy( info = clsInfo.derivedClassInfo( decls = clsInfo.decls.filteredScope(!dropped.contains(_)))) + + // TODO: this happens to work only because Constructors is the last phase in group } val (superCalls, followConstrStats) = constrStats.toList match { diff --git a/src/dotty/tools/dotc/transform/Memoize.scala b/src/dotty/tools/dotc/transform/Memoize.scala index 728005cab..63466dc46 100644 --- a/src/dotty/tools/dotc/transform/Memoize.scala +++ b/src/dotty/tools/dotc/transform/Memoize.scala @@ -70,16 +70,16 @@ import Decorators._ lazy val field = sym.field.orElse(newField).asTerm if (sym.is(Accessor, butNot = NoFieldNeeded)) if (sym.isGetter) { - var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform) - if (isWildcardArg(rhs)) rhs = EmptyTree - val fieldDef = transformFollowing(ValDef(field, rhs)) - val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))) - Thicket(fieldDef, getterDef) - } + var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform) + if (isWildcardArg(rhs)) rhs = EmptyTree + val fieldDef = transformFollowing(ValDef(field, rhs)) + val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))(ctx.withOwner(sym), info)) + Thicket(fieldDef, getterDef) + } else if (sym.isSetter) { if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs } // this is intended as an assertion val initializer = Assign(ref(field), ref(tree.vparamss.head.head.symbol)) - cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)) + cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(ctx.withOwner(sym), info)) } else tree // curiously, some accessors from Scala2 have ' ' suffixes. They count as // neither getters nor setters |