summaryrefslogtreecommitdiff
path: root/src/compiler/scala/tools/nsc/transform/Constructors.scala
diff options
context:
space:
mode:
authorAdriaan Moors <adriaan.moors@typesafe.com>2015-09-01 14:48:07 -0700
committerAdriaan Moors <adriaan.moors@typesafe.com>2015-11-11 15:52:33 -0800
commite77698fb94ff679f3bf52c23c86562156c3f7a78 (patch)
tree037409064dee0aec6d99bd37e26aed49e77d2b43 /src/compiler/scala/tools/nsc/transform/Constructors.scala
parent086e66257a511a90ecd40e7edf79aadd64ac65b2 (diff)
downloadscala-e77698fb94ff679f3bf52c23c86562156c3f7a78.tar.gz
scala-e77698fb94ff679f3bf52c23c86562156c3f7a78.tar.bz2
scala-e77698fb94ff679f3bf52c23c86562156c3f7a78.zip
Constructors: mutate less
Codify the scope of mutability for various buffers, inline one-time methods, further reduce spooky action at a distance.
Diffstat (limited to 'src/compiler/scala/tools/nsc/transform/Constructors.scala')
-rw-r--r--src/compiler/scala/tools/nsc/transform/Constructors.scala210
1 files changed, 98 insertions, 112 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala
index 46f93b81e8..24e5c12b12 100644
--- a/src/compiler/scala/tools/nsc/transform/Constructors.scala
+++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala
@@ -6,8 +6,7 @@
package scala.tools.nsc
package transform
-import scala.collection.{ mutable, immutable }
-import scala.collection.mutable.ListBuffer
+import scala.collection.mutable
import scala.reflect.internal.util.ListOfNil
import symtab.Flags._
@@ -120,15 +119,15 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
* What trees can be visited at this point?
* To recap, by the time the constructors phase runs, local definitions have been hoisted out of their original owner.
* Moreover, by the time elision is about to happen, the `intoConstructors` rewriting
- * of template-level statements has taken place (the resulting trees can be found in `constrStatBuf`).
+ * of template-level statements has taken place (the resulting trees can be found in `constructorStats`).
*
* That means:
*
- * - nested classes are to be found in `defBuf`
+ * - nested classes are to be found in `defs`
*
- * - value and method definitions are also in `defBuf` and none of them contains local methods or classes.
+ * - value and method definitions are also in `defs` and none of them contains local methods or classes.
*
- * - auxiliary constructors are to be found in `auxConstructorBuf`
+ * - auxiliary constructors are to be found in `auxConstructors`
*
* Coming back to the question which trees may contain accesses:
*
@@ -148,57 +147,44 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
*
*/
private trait OmittablesHelper {
- def computeOmittableAccessors(clazz: Symbol, defBuf: ListBuffer[Tree], auxConstructorBuf: ListBuffer[Tree]): Set[Symbol] = {
- /*
- * Initially populated with all elision candidates.
- * Trees are traversed, and those candidates are removed which are actually needed.
- * After that, `omittables` doesn't shrink anymore: each symbol it contains can be unlinked from clazz.info.decls.
- */
- val omittables = mutable.Set.empty[Symbol]
+ def computeOmittableAccessors(clazz: Symbol, defs: List[Tree], auxConstructors: List[Tree]): Set[Symbol] = {
+ val decls = clazz.info.decls.toSet
+ val isEffectivelyFinal = clazz.isEffectivelyFinal
+ // Initially populated with all elision candidates.
+ // Trees are traversed, and those candidates are removed which are actually needed.
+ // After that, `omittables` doesn't shrink anymore: each symbol it contains can be unlinked from clazz.info.decls.
+ //
// Note: elision of outer reference is based on a class-wise analysis, if a class might have subclasses,
// it doesn't work. For example, `LocalParent` retains the outer reference in:
//
// class Outer { def test = {class LocalParent; class LocalChild extends LocalParent } }
//
// See run/t9408.scala for related test cases.
- val isEffectivelyFinal = clazz.isEffectivelyFinal
- def isParamCandidateForElision(sym: Symbol) = (sym.isParamAccessor && sym.isPrivateLocal)
- def isOuterCandidateForElision(sym: Symbol) = (sym.isOuterAccessor && isEffectivelyFinal && !sym.isOverridingSymbol)
-
- val decls = clazz.info.decls.toSet
- val paramCandidatesForElision: Set[ /*Field*/ Symbol] = (decls filter isParamCandidateForElision)
- val outerCandidatesForElision: Set[ /*Method*/ Symbol] = (decls filter isOuterCandidateForElision)
-
- omittables ++= paramCandidatesForElision
- omittables ++= outerCandidatesForElision
-
- val bodyOfOuterAccessor: Map[Symbol, DefDef] =
- defBuf.collect { case dd: DefDef if outerCandidatesForElision(dd.symbol) => dd.symbol -> dd }.toMap
+ def omittableParamAcc(sym: Symbol) = sym.isParamAccessor && sym.isPrivateLocal
+ def omittableOuterAcc(sym: Symbol) = isEffectivelyFinal && sym.isOuterAccessor && !sym.isOverridingSymbol
+ val omittables = mutable.Set.empty[Symbol] ++ (decls filter (sym => omittableParamAcc(sym) || omittableOuterAcc(sym))) // the closure only captures isEffectivelyFinal
// no point traversing further once omittables is empty, all candidates ruled out already.
object detectUsages extends Traverser {
- private def markUsage(sym: Symbol) {
- omittables -= debuglogResult("omittables -= ")(sym)
- // recursive call to mark as needed the field supporting the outer-accessor-method.
- bodyOfOuterAccessor get sym foreach (this traverse _.rhs)
- }
- override def traverse(tree: Tree): Unit = if (omittables.nonEmpty) {
- def sym = tree.symbol
- tree match {
- // don't mark as "needed" the field supporting this outer-accessor, ie not just yet.
- case _: DefDef if outerCandidatesForElision(sym) => ()
- case _: Select if omittables(sym) => markUsage(sym) ; super.traverse(tree)
- case _ => super.traverse(tree)
+ lazy val bodyOfOuterAccessor = defs collect { case dd: DefDef if omittableOuterAcc(dd.symbol) => dd.symbol -> dd.rhs } toMap
+
+ override def traverse(tree: Tree): Unit =
+ if (omittables.nonEmpty) {
+ def sym = tree.symbol
+ tree match {
+ case _: DefDef if (sym.owner eq clazz) && omittableOuterAcc(sym) => // don't mark as "needed" the field supporting this outer-accessor (not just yet)
+ case _: Select if omittables(sym) => omittables -= sym // mark usage
+ bodyOfOuterAccessor get sym foreach traverse // recurse to mark as needed the field supporting the outer-accessor-method
+ super.traverse(tree)
+ case _ => super.traverse(tree)
+ }
}
- }
- def walk(xs: Seq[Tree]) = xs.iterator foreach traverse
- }
- if (omittables.nonEmpty) {
- detectUsages walk defBuf
- detectUsages walk auxConstructorBuf
}
+ if (omittables.nonEmpty)
+ (defs.iterator ++ auxConstructors.iterator) foreach detectUsages.traverse
+
omittables.toSet
}
} // OmittablesHelper
@@ -255,8 +241,6 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
*
* */
private trait DelayedInitHelper extends ConstructorTransformerBase {
- lazy val isDelayedInitSubclass = clazz isSubClass DelayedInitClass
-
private def delayedEndpointDef(stats: List[Tree]): DefDef = {
val methodName = currentUnit.freshTermName("delayedEndpoint$" + clazz.fullNameAsName('$').toString + "$")
val methodSym = clazz.newMethod(methodName, impl.pos, SYNTHETIC | FINAL)
@@ -316,31 +300,27 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
satelliteClass.asInstanceOf[ClassDef]
}
- private def delayedInitCall(closure: Tree) = localTyper.typedPos(impl.pos) {
- gen.mkMethodCall(This(clazz), delayedInitMethod, Nil, List(New(closure.symbol.tpe, This(clazz))))
- }
+ /** For a DelayedInit subclass, wrap remainingConstrStats into a DelayedInit closure.
+ *
+ * TODO: XXX This condition (`isDelayedInitSubclass && remainingConstrStats.nonEmpty`) is not correct:
+ * remainingConstrStats.nonEmpty excludes too much,
+ * but excluding it includes too much. The constructor sequence being mimicked
+ * needs to be reproduced with total fidelity.
+ *
+ * See test case files/run/bug4680.scala, the output of which is wrong in many
+ * particulars.
+ */
+ def delayedInitDefsAndConstrStats(defs: List[Tree], remainingConstrStats: List[Tree]): (List[Tree], List[Tree]) = {
+ val delayedHook = delayedEndpointDef(remainingConstrStats)
+ val delayedHookSym = delayedHook.symbol.asInstanceOf[MethodSymbol]
+
+ // transform to make the closure-class' default constructor assign the the outer instance to its param-accessor field.
+ val hookCallerClass = (new ConstructorTransformer(unit)) transform delayedInitClosure(delayedHookSym)
+ val delayedInitCall = localTyper.typedPos(impl.pos) {
+ gen.mkMethodCall(This(clazz), delayedInitMethod, Nil, List(New(hookCallerClass.symbol.tpe, This(clazz))))
+ }
- def rewriteDelayedInit(defBuf: ListBuffer[Tree], remainingConstrStats: List[Tree]): List[Tree] = {
- /* XXX This is not correct: remainingConstrStats.nonEmpty excludes too much,
- * but excluding it includes too much. The constructor sequence being mimicked
- * needs to be reproduced with total fidelity.
- *
- * See test case files/run/bug4680.scala, the output of which is wrong in many
- * particulars.
- */
- val needsDelayedInit = (isDelayedInitSubclass && remainingConstrStats.nonEmpty)
-
- if (needsDelayedInit) {
- val delayedHook: DefDef = delayedEndpointDef(remainingConstrStats)
- defBuf += delayedHook
- val hookCallerClass = {
- // transform to make the closure-class' default constructor assign the the outer instance to its param-accessor field.
- val drillDown = new ConstructorTransformer(unit)
- drillDown transform delayedInitClosure(delayedHook.symbol.asInstanceOf[MethodSymbol])
- }
- defBuf += hookCallerClass
- delayedInitCall(hookCallerClass) :: Nil
- } else remainingConstrStats
+ (List(delayedHook, hookCallerClass), List(delayedInitCall))
}
} // DelayedInitHelper
@@ -360,7 +340,7 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
* `specializedStats` are replaced by the specialized assignment.
*/
private def mergeConstructors(genericClazz: Symbol, originalStats: List[Tree], specializedStats: List[Tree]): List[Tree] = {
- val specBuf = new ListBuffer[Tree]
+ val specBuf = new mutable.ListBuffer[Tree]
specBuf ++= specializedStats
def specializedAssignFor(sym: Symbol): Option[Tree] =
@@ -479,6 +459,9 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
val clazz = impl.symbol.owner // the transformed class
val localTyper = typer.atOwner(impl, clazz)
+
+ val isDelayedInitSubclass = clazz isSubClass DelayedInitClass
+
private val stats = impl.body // the transformed template body
// find and dissect primary constructor
@@ -585,33 +568,28 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
val result = mkAssign(to, Ident(from))
if (from.name != nme.OUTER ||
- from.tpe.typeSymbol.isPrimitiveValueClass) result
+ from.tpe.typeSymbol.isPrimitiveValueClass) result
else localTyper.typedPos(to.pos) {
// `throw null` has the same effect as `throw new NullPointerException`, see JVM spec on instruction `athrow`
- IF (from OBJ_EQ NULL) THEN Throw(gen.mkZero(ThrowableTpe)) ELSE result
+ IF(from OBJ_EQ NULL) THEN Throw(gen.mkZero(ThrowableTpe)) ELSE result
}
}
- def transformed = {
- // The list of definitions that go into class
- val defBuf = new ListBuffer[Tree]
-
- // The auxiliary constructors, separate from the defBuf since they should
- // follow the primary constructor
- val auxConstructorBuf = new ListBuffer[Tree]
-
- // The list of statements that go into the constructor after and including the superclass constructor call
- val constrStatBuf = new ListBuffer[Tree]
-
- // The list of early initializer statements that go into constructor before the superclass constructor call
- val constrPrefixBuf = new ListBuffer[Tree]
+ /** Triage definitions and statements in this template into the following categories.
+ * The primary constructor is treated separately, as it is assembled in part from these pieces.
+ *
+ * - `defs`: definitions that go into class
+ * - `auxConstrs`: auxiliary constructors, separate from the defs as they should follow the primary constructor
+ * - `constrPrefix`: early initializer statements that go into constructor before the superclass constructor call
+ * - `constrStats`: statements that go into the constructor after and including the superclass constructor call
+ * - `classInitStats`: statements that go into the class initializer
+ */
+ def triageStats = {
+ val defBuf, auxConstructorBuf, constrPrefixBuf, constrStatBuf, classInitStatBuf = new mutable.ListBuffer[Tree]
// The early initialized field definitions of the class (these are the class members)
val presupers = treeInfo.preSuperFields(stats)
- // The list of statements that go into the class initializer
- val classInitStatBuf = new ListBuffer[Tree]
-
// generate code to copy pre-initialized fields
for (stat <- primaryConstrBody.stats) {
constrStatBuf += stat
@@ -623,11 +601,10 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
val to = fields.head.symbol
if (!to.tpe.isInstanceOf[ConstantType])
constrStatBuf += mkAssign(to, Ident(stat.symbol))
- case _ =>
+ case _ =>
}
}
-
for (stat <- stats) {
val statSym = stat.symbol
@@ -662,8 +639,8 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
// regular ones go to `defBuf`, secondary contructors go to `auxConstructorBuf`.
// The primary constructor is dealt with separately (we're massaging it here).
case _: DefDef if statSym.isPrimaryConstructor => ()
- case _: DefDef if statSym.isConstructor => auxConstructorBuf += stat
- case _: DefDef => defBuf += stat
+ case _: DefDef if statSym.isConstructor => auxConstructorBuf += stat
+ case _: DefDef => defBuf += stat
// If a val needs a field, an empty valdef goes into the template.
// Except for lazy and ConstantTyped vals, the field is initialized by an assignment in:
@@ -680,19 +657,25 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
}
}
+ (defBuf.toList, auxConstructorBuf.toList, constrPrefixBuf.toList, constrStatBuf.toList, classInitStatBuf.toList)
+ }
+ def transformed = {
+ val (defs, auxConstructors, constructorPrefix, constructorStats, classInitStats) = triageStats
+
+ // omit unused outers
val omittableAccessor: Set[Symbol] =
if (isDelayedInitSubclass) Set.empty
- else computeOmittableAccessors(clazz, defBuf, auxConstructorBuf)
+ else computeOmittableAccessors(clazz, defs, auxConstructors)
- // omit unused outers
- def omittable(sym: Symbol) = omittableAccessor(sym)
+ def omittableSym(sym: Symbol) = omittableAccessor(sym)
+ def omittableStat(stat: Tree) = omittableSym(stat.symbol)
// The parameter accessor fields which are members of the class
val paramAccessors = clazz.constrParamAccessors
// Initialize all parameters fields that must be kept.
- val paramInits = paramAccessors filterNot omittable map { acc =>
+ val paramInits = paramAccessors filterNot omittableSym map { acc =>
// Check for conflicting symbol amongst parents: see bug #1960.
// It would be better to mangle the constructor parameter name since
// it can only be used internally, but I think we need more robust name
@@ -704,40 +687,43 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
copyParam(acc, parameter(acc))
}
- /* Return a pair consisting of (all statements up to and including superclass and trait constr calls, rest) */
+ // Return a pair consisting of (all statements up to and including superclass and trait constr calls, rest)
def splitAtSuper(stats: List[Tree]) = {
def isConstr(tree: Tree): Boolean = tree match {
case Block(_, expr) => isConstr(expr) // SI-6481 account for named argument blocks
case _ => (tree.symbol ne null) && tree.symbol.isConstructor
}
- val (pre, rest0) = stats span (!isConstr(_))
+ val (pre, rest0) = stats span (!isConstr(_))
val (supercalls, rest) = rest0 span (isConstr(_))
(pre ::: supercalls, rest)
}
- val (uptoSuperStats, remainingConstrStats0) = splitAtSuper(constrStatBuf.toList)
- val remainingConstrStats = rewriteDelayedInit(defBuf, remainingConstrStats0)
+ val (uptoSuperStats, remainingConstrStats) = splitAtSuper(constructorStats)
+ val (delayedHookDefs, remainingConstrStatsDelayedInit) =
+ if (isDelayedInitSubclass && remainingConstrStats.nonEmpty) delayedInitDefsAndConstrStats(defs, remainingConstrStats)
+ else (Nil, remainingConstrStats)
// Assemble final constructor
- defBuf += deriveDefDef(primaryConstr)(_ =>
+ val primaryConstructor = deriveDefDef(primaryConstr)(_ => {
treeCopy.Block(
primaryConstrBody,
- paramInits ::: constrPrefixBuf.toList ::: uptoSuperStats :::
- guardSpecializedInitializer(remainingConstrStats),
- primaryConstrBody.expr))
+ paramInits ::: constructorPrefix ::: uptoSuperStats ::: guardSpecializedInitializer(remainingConstrStatsDelayedInit),
+ primaryConstrBody.expr)
+ })
- // Followed by any auxiliary constructors
- defBuf ++= auxConstructorBuf
+ val constructors = primaryConstructor :: auxConstructors
// Unlink all fields that can be dropped from class scope
- for (sym <- clazz.info.decls; if omittable(sym))
- clazz.info.decls unlink sym
+ // Iterating on toList is cheaper (decls.filter does a toList anyway)
+ val decls = clazz.info.decls
+ decls.toList.filter(omittableSym).foreach(decls.unlink)
- // Eliminate all field definitions that can be dropped from template
- val templateWithoutOmittables: Template = deriveTemplate(impl)(_ => defBuf.toList filterNot (stat => omittable(stat.symbol)))
+ // Eliminate all field/accessor definitions that can be dropped from template
+ // We never eliminate delayed hooks or the constructors, so, only filter `defs`.
+ val prunedStats = (defs filterNot omittableStat) ::: delayedHookDefs ::: constructors
// Add the static initializers
- addStaticInits(templateWithoutOmittables, classInitStatBuf, localTyper)
+ addStaticInits(deriveTemplate(impl)(_ => prunedStats), classInitStats, localTyper)
}
} // TemplateTransformer
}