omit unneeded fields (for constants)

Author: adriaanm

src/compiler/scala/tools/nsc/transform/Constructors.scala

@@ -220,7 +220,9 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
         detectUsages walk auxConstructorBuf
       }
     }
-    def mustBeKept(sym: Symbol) = !(omittables(sym) || (sym.owner.isTrait && sym.isValue && !sym.isMethod))
+
+    def needsField(sym: Symbol): Boolean = typeNeedsStorage(enteringErasure(sym.info).resultType)
+    def mustBeKept(sym: Symbol) = !(omittables(sym) || (sym.isValue && !sym.isMethod && (sym.owner.isTrait || !needsField(sym)))) // TODO: assert(!sym.owner.isTrait, sym.ownerChain)
 
   } // OmittablesHelper
 
@@ -597,24 +599,41 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
     def intoConstructor(oldowner: Symbol, tree: Tree) =
       intoConstructorTransformer transform tree.changeOwner(oldowner -> constr.symbol)
 
-    private def assignmentIntoCtor(stat: ValOrDefDef): Unit = {
-      val rhs1 = intoConstructor(stat.symbol, stat.rhs)
+    private def needsCtorEffect(stat: ValOrDefDef): Boolean = !(stat.rhs == EmptyTree || stat.symbol.isLazy)
+
+    // Move the RHS of a Val/Def (traits don't have vals, their getter holds the RHS) to the appropriate part of the ctor.
+    // If the val def is an early initialized or a parameter accessor,
+    // it goes before the superclass constructor call, otherwise it goes after.
+    // A lazy val's effect is not moved to the constructor, as it is delayed.
+    private def moveEffectToCtor(stat: ValOrDefDef): Boolean = {
+      val fieldNeeded = needsField(stat.symbol)
+
+      if (needsCtorEffect(stat)) {
+        val rhs1 = intoConstructor(stat.symbol, stat.rhs)
 
-      // Should tree be moved in front of super constructor call?
-      // (I.e., is it derived from an early definition?)
-      val constrBufPart =
-        if (stat.mods hasFlag PRESUPER | PARAMACCESSOR) constrPrefixBuf
-        else constrStatBuf
+        // Should tree be moved in front of super constructor call?
+        // (I.e., is it derived from an early definition?)
+        val constrBufPart =
+          if (stat.mods hasFlag PRESUPER | PARAMACCESSOR) constrPrefixBuf
+          else constrStatBuf
 
-      val assignSym =
-        if (stat.symbol.isGetter) stat.symbol.asInstanceOf[MethodSymbol].referenced //setterIn(clazz, hasExpandedName = false) // not yet expanded -- see Mixin
-        else stat.symbol
+        val effect =
+          if (fieldNeeded) {
+            val assignSym =
+              if (stat.symbol.isGetter) stat.symbol.asInstanceOf[MethodSymbol].referenced //setterIn(clazz, hasExpandedName = false) // not yet expanded -- see Mixin
+              else stat.symbol
 
-      assert(assignSym ne NoSymbol, stat)
+            assert(assignSym ne NoSymbol, stat)
+            mkAssign(assignSym, rhs1)
+          } else rhs1
+
+        constrBufPart += effect
+      }
 
-      constrBufPart += mkAssign(assignSym, rhs1)
+      fieldNeeded
     }
 
+
     // TODO: we probably can't emit an Assign tree after typers, need to Apply the setter
     // Create an assignment to class field `to` with rhs `from`
     def mkAssign(to: Symbol, from: Tree): Tree =
@@ -692,24 +711,20 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
             else if (stat.symbol.isConstructor) auxConstructorBuf += stat
             // a concrete getter's RHS is treated like a ValDef (the actual field isn't emitted until Mixin augments the class that inherits the trait)
             else if (inTrait && stat.symbol.isGetter && (rhs ne EmptyTree)) {
-              assignmentIntoCtor(dd)
+              moveEffectToCtor(dd) // ignore whether we need a field -- it will be added in the inheriting class by Mixin
               defBuf += deriveDefDef(dd)(_ => EmptyTree)
             }
             else defBuf += stat
         }
       case vd@ 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
-        // if the val def is an early initialized or a parameter accessor, it goes
-        // before the superclass constructor call, otherwise it goes after.
-        // Lazy vals don't get the assignment in the constructor.
-        if (!stat.symbol.tpe.isInstanceOf[ConstantType]) { // TODO: MethodSynthesis should no longer emit ValDefs with ConstantTypes
-          if (rhs != EmptyTree && !stat.symbol.isLazy) {
-            assignmentIntoCtor(vd)
-          }
-          defBuf += deriveValDef(stat)(_ => EmptyTree)
-        }
+        // Move field's effect into the constructor (usually an assignment of rhs to field,
+        // but it could also be the side-effect of the rhs when no storage is needed).
+        val needsField = moveEffectToCtor(vd)
+        println(s"$vd needsField: $needsField")
+
+        // Only emit fields that actually need to be stored
+        if (needsField) 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)

src/compiler/scala/tools/nsc/transform/Mixin.scala

@@ -266,7 +266,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
 
             // mixin one field only (this method is called for both accessors)
             // don't mix in a field at all if it doesn't need storage
-            if (!(traitMember.isSetter || typeNeedsNoStorage(traitMember.tpe.resultType)))
+            if (traitMember.isGetter && typeNeedsStorage(traitMember.tpe.resultType))
               mixinFieldSym(traitMember)
           }
           else devWarning(s"Overridden concrete accessor: ${traitMember.fullLocationString}")

src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala

@@ -134,7 +134,8 @@ trait MethodSynthesis {
       ImplicitClassWrapper(tree).createAndEnterSymbol()
     }
 
-    // TODO: see standardAccessors
+    // TODO: why is all this symbol creation disconnected from addDerivedTrees,
+    // which creates another list of Field/Getter/Setter factories???
     def enterGetterSetter(tree: ValDef) {
       val ValDef(mods, name, _, _) = tree
       if (nme.isSetterName(name))
@@ -321,7 +322,7 @@ trait MethodSynthesis {
       //     Constructors will move the assignment to the constructor, abstracting over the field using the field setter,
       //     and Mixin will add a field to the class that mixes in the trait, implementing the accessors in terms of it
       def noFieldHere = nothingToMemoize || owner.isTrait
-      protected def nothingToMemoize = isDeferred || typeNeedsNoStorage(basisSym.tpe.resultType)
+      protected def nothingToMemoize = isDeferred // TODO: we don't know the val's type at this point... || typeNeedsNoStorage(???)
 
       def isSetter   = false
       def isDeferred = mods.isDeferred

src/reflect/scala/reflect/internal/Definitions.scala

@@ -236,7 +236,7 @@ trait Definitions extends api.StandardDefinitions {
     // Take into account annotations so that we keep annotated unit lazy val
     // to get better error message already from the cps plugin itself
     def isUnitType(tp: Type) = tp.typeSymbol == UnitClass && tp.annotations.isEmpty // you never know with annotations...
-    def typeNeedsNoStorage(tp: Type) = tp.isInstanceOf[ConstantType] || isUnitType(tp)
+    def typeNeedsStorage(tp: Type) = !(tp.isInstanceOf[ConstantType] || isUnitType(tp))
 
     def hasMultipleNonImplicitParamLists(member: Symbol): Boolean = hasMultipleNonImplicitParamLists(member.info)
     def hasMultipleNonImplicitParamLists(info: Type): Boolean = info match {