From f7f80e8b27c9255590ef8598c97601957a962adf Mon Sep 17 00:00:00 2001 From: Simon Ochsenreither Date: Sat, 23 Nov 2013 22:45:40 +0100 Subject: [PATCH] SI-7971 Handle static field initializers correctly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this fix, static fields where erroneously treated like instance fields and the initialization was moved into the constructor. With this fix, the static initializer statements go into the static initializer of the class (called “ def init” in Scala, in Java). The statements are added to an existing static initializer method or, if no such method exists, a new static initializer method is created and added to the class. --- .../scala/tools/nsc/transform/CleanUp.scala | 44 ++-------------- .../tools/nsc/transform/Constructors.scala | 26 +++++++--- .../scala/tools/nsc/transform/Statics.scala | 52 +++++++++++++++++++ 3 files changed, 73 insertions(+), 49 deletions(-) create mode 100644 src/compiler/scala/tools/nsc/transform/Statics.scala diff --git a/src/compiler/scala/tools/nsc/transform/CleanUp.scala b/src/compiler/scala/tools/nsc/transform/CleanUp.scala index 3ecce8d7b1df..9738769db95b 100644 --- a/src/compiler/scala/tools/nsc/transform/CleanUp.scala +++ b/src/compiler/scala/tools/nsc/transform/CleanUp.scala @@ -11,7 +11,7 @@ import Flags._ import scala.collection._ import scala.language.postfixOps -abstract class CleanUp extends Transform with ast.TreeDSL { +abstract class CleanUp extends Statics with Transform with ast.TreeDSL { import global._ import definitions._ import CODE._ @@ -35,7 +35,7 @@ abstract class CleanUp extends Transform with ast.TreeDSL { protected def newTransformer(unit: CompilationUnit): Transformer = new CleanUpTransformer(unit) - class CleanUpTransformer(unit: CompilationUnit) extends Transformer { + class CleanUpTransformer(unit: CompilationUnit) extends StaticsTransformer { private val newStaticMembers = mutable.Buffer.empty[Tree] private val newStaticInits = mutable.Buffer.empty[Tree] private val symbolsStoredAsStatic = mutable.Map.empty[String, Symbol] @@ -49,7 +49,7 @@ abstract class CleanUp extends Transform with ast.TreeDSL { clearStatics() val newBody = transformTrees(body) val templ = deriveTemplate(tree)(_ => transformTrees(newStaticMembers.toList) ::: newBody) - try addStaticInits(templ) // postprocess to include static ctors + try addStaticInits(templ, newStaticInits, localTyper) // postprocess to include static ctors finally clearStatics() } private def mkTerm(prefix: String): TermName = unit.freshTermName(prefix) @@ -557,44 +557,6 @@ abstract class CleanUp extends Transform with ast.TreeDSL { }) } - /* finds the static ctor DefDef tree within the template if it exists. */ - private def findStaticCtor(template: Template): Option[Tree] = - template.body find { - case defdef @ DefDef(_, nme.CONSTRUCTOR, _, _, _, _) => defdef.symbol.hasStaticFlag - case _ => false - } - - /* changes the template for the class so that it contains a static constructor with symbol fields inits, - * augments an existing static ctor if one already existed. - */ - private def addStaticInits(template: Template): Template = { - if (newStaticInits.isEmpty) - template - else { - val newCtor = findStaticCtor(template) match { - // in case there already were static ctors - augment existing ones - // currently, however, static ctors aren't being generated anywhere else - case Some(ctor @ DefDef(_,_,_,_,_,_)) => - // modify existing static ctor - deriveDefDef(ctor) { - case block @ Block(stats, expr) => - // need to add inits to existing block - treeCopy.Block(block, newStaticInits.toList ::: stats, expr) - case term: TermTree => - // need to create a new block with inits and the old term - treeCopy.Block(term, newStaticInits.toList, term) - } - case _ => - // create new static ctor - val staticCtorSym = currentClass.newStaticConstructor(template.pos) - val rhs = Block(newStaticInits.toList, Literal(Constant(()))) - - localTyper.typedPos(template.pos)(DefDef(staticCtorSym, rhs)) - } - deriveTemplate(template)(newCtor :: _) - } - } - } // CleanUpTransformer } diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala index b97b1e35273a..391bce5abbf6 100644 --- a/src/compiler/scala/tools/nsc/transform/Constructors.scala +++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala @@ -13,7 +13,7 @@ import symtab.Flags._ /** This phase converts classes with parameters into Java-like classes with * fields, which are assigned to from constructors. */ -abstract class Constructors extends Transform with ast.TreeDSL { +abstract class Constructors extends Statics with Transform with ast.TreeDSL { import global._ import definitions._ @@ -199,7 +199,7 @@ abstract class Constructors extends Transform with ast.TreeDSL { detectUsages walk auxConstructorBuf } } - def mustbeKept(sym: Symbol) = !omittables(sym) + def mustBeKept(sym: Symbol) = !omittables(sym) } // OmittablesHelper @@ -458,7 +458,7 @@ abstract class Constructors extends Transform with ast.TreeDSL { } // GuardianOfCtorStmts private class TemplateTransformer(val unit: CompilationUnit, val impl: Template) - extends Transformer + extends StaticsTransformer with DelayedInitHelper with OmittablesHelper with GuardianOfCtorStmts { @@ -607,7 +607,7 @@ abstract class Constructors extends Transform with ast.TreeDSL { // follow the primary constructor val auxConstructorBuf = new ListBuffer[Tree] - // The list of statements that go into constructor after and including the superclass constructor call + // 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 @@ -616,6 +616,9 @@ abstract class Constructors extends Transform with ast.TreeDSL { // 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 <- constrBody.stats) { constrStatBuf += stat @@ -644,7 +647,7 @@ abstract class Constructors extends Transform with ast.TreeDSL { else if (stat.symbol.isConstructor) auxConstructorBuf += stat else defBuf += stat } - case ValDef(_, _, _, rhs) => + case ValDef(mods, _, _, rhs) if !mods.hasStaticFlag => // val defs with constant right-hand sides are eliminated. // for all other val defs, an empty valdef goes into the template and // the initializer goes as an assignment into the constructor @@ -659,6 +662,11 @@ abstract class Constructors extends Transform with ast.TreeDSL { } defBuf += deriveValDef(stat)(_ => EmptyTree) } + case ValDef(_, _, _, rhs) => + // Add static initializer statements to classInitStatBuf and remove the rhs from the val def. + classInitStatBuf += mkAssign(stat.symbol, rhs) + defBuf += deriveValDef(stat)(_ => EmptyTree) + case ClassDef(_, _, _, _) => // classes are treated recursively, and left in the template defBuf += new ConstructorTransformer(unit).transform(stat) @@ -670,7 +678,7 @@ abstract class Constructors extends Transform with ast.TreeDSL { populateOmittables() // Initialize all parameters fields that must be kept. - val paramInits = paramAccessors filter mustbeKept map { acc => + val paramInits = paramAccessors filter mustBeKept 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 @@ -710,11 +718,13 @@ abstract class Constructors extends Transform with ast.TreeDSL { defBuf ++= auxConstructorBuf // Unlink all fields that can be dropped from class scope - for (sym <- clazz.info.decls ; if !mustbeKept(sym)) + for (sym <- clazz.info.decls ; if !mustBeKept(sym)) clazz.info.decls unlink sym // Eliminate all field definitions that can be dropped from template - val transformed: Template = deriveTemplate(impl)(_ => defBuf.toList filter (stat => mustbeKept(stat.symbol))) + val templateWithoutOmittables: Template = deriveTemplate(impl)(_ => defBuf.toList filter (stat => mustBeKept(stat.symbol))) + // Add the static initializers + val transformed: Template = addStaticInits(templateWithoutOmittables, classInitStatBuf, localTyper) } // TemplateTransformer diff --git a/src/compiler/scala/tools/nsc/transform/Statics.scala b/src/compiler/scala/tools/nsc/transform/Statics.scala new file mode 100644 index 000000000000..e2508b8d08df --- /dev/null +++ b/src/compiler/scala/tools/nsc/transform/Statics.scala @@ -0,0 +1,52 @@ +package scala.tools.nsc +package transform + +import symtab._ +import Flags._ + +import collection.mutable.Buffer + +abstract class Statics extends Transform with ast.TreeDSL { + import global._ + + class StaticsTransformer extends Transformer { + + /** finds the static ctor DefDef tree within the template if it exists. */ + def findStaticCtor(template: Template): Option[Tree] = + template.body find { + case defdef @ DefDef(_, nme.CONSTRUCTOR, _, _, _, _) => defdef.symbol.hasStaticFlag + case _ => false + } + + /** changes the template for the class so that it contains a static constructor with symbol fields inits, + * augments an existing static ctor if one already existed. + */ + def addStaticInits(template: Template, newStaticInits: Buffer[Tree], localTyper: analyzer.Typer): Template = { + if (newStaticInits.isEmpty) + template + else { + val newCtor = findStaticCtor(template) match { + // in case there already were static ctors - augment existing ones + // currently, however, static ctors aren't being generated anywhere else + case Some(ctor @ DefDef(_,_,_,_,_,_)) => + // modify existing static ctor + deriveDefDef(ctor) { + case block @ Block(stats, expr) => + // need to add inits to existing block + treeCopy.Block(block, newStaticInits.toList ::: stats, expr) + case term: TermTree => + // need to create a new block with inits and the old term + treeCopy.Block(term, newStaticInits.toList, term) + } + case _ => + // create new static ctor + val staticCtorSym = currentClass.newStaticConstructor(template.pos) + val rhs = Block(newStaticInits.toList, Literal(Constant(()))) + + localTyper.typedPos(template.pos)(DefDef(staticCtorSym, rhs)) + } + deriveTemplate(template)(newCtor :: _) + } + } + } +}