do assignment to trait fields in AddInterfaces

regular class vals get assignments during constructors, as before

Author: adriaanm

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

@@ -58,8 +58,9 @@ abstract class AddInterfaces extends InfoTransform { self: Erasure =>
   def needsImplMethod(sym: Symbol) = (
        sym.isMethod
     && isInterfaceMember(sym)
-    && (!(sym hasFlag ACCESSOR) || sym.isLazy || !(sym hasFlag SYNTHESIZE_IMPL_IN_SUBCLASS)) // SYNTHESIZE_IMPL_IN_SUBCLASS accessors are mixed in by the fields phase, but others should be treated as regulars methods (constant-typed getters)
-// so that constructors populates the init method in the impl class
+       // SYNTHESIZE_IMPL_IN_SUBCLASS accessors are mixed in by the fields phase, but others should be treated as regulars methods
+       // (this will eventually include constant-typed getters, as they can be fully implemented in the interface)
+    && (!(sym hasFlag ACCESSOR) || sym.isLazy || !(sym hasFlag SYNTHESIZE_IMPL_IN_SUBCLASS))
     && (!sym.hasFlag(DEFERRED | SUPERACCESSOR) || (sym hasFlag lateDEFERRED))
   )
 
@@ -245,13 +246,25 @@ abstract class AddInterfaces extends InfoTransform { self: Erasure =>
   }
 
   private def isInterfaceTree(tree: Tree) = tree.isDef && isInterfaceMember(tree.symbol)
+  private def isMemoizedTraitGetter(tree: Tree) = (tree.symbol hasFlag SYNTHESIZE_IMPL_IN_SUBCLASS) && tree.symbol.isGetter && !tree.symbol.info.resultType.isInstanceOf[ConstantType]
 
-  private def deriveMemberForImplClass(tree: Tree): Tree  =
-    if (isInterfaceTree(tree)) if (needsImplMethod(tree.symbol)) implMethodDef(tree)            else EmptyTree
+  private def mkAssign(clazz: Symbol, assignSym: Symbol, rhs: Tree): Tree = {
+    val qual = Select(This(clazz), assignSym)
+    if (assignSym.isSetter) Apply(qual, List(rhs))
+    else Assign(qual, rhs)
+  }
+
+  private def deriveMemberForImplClass(clazz: Symbol, iface: Symbol, exprOwner: Symbol)(tree: Tree): Tree  =
+    if (isInterfaceTree(tree))
+      if (needsImplMethod(tree.symbol)) implMethodDef(tree)
+      else if (isMemoizedTraitGetter(tree)) mkAssign(clazz, tree.symbol.setterIn(iface), tree.asInstanceOf[DefDef].rhs.changeOwner(tree.symbol -> exprOwner))
+      else EmptyTree
     else tree
 
   private def deriveMemberForInterface(tree: Tree): Tree =
-    if (isInterfaceTree(tree)) if (needsImplMethod(tree.symbol)) DefDef(tree.symbol, EmptyTree) else tree
+    if (isInterfaceTree(tree))
+      if (needsImplMethod(tree.symbol) || isMemoizedTraitGetter(tree)) DefDef(tree.symbol, EmptyTree) // TODO: use deriveDefDef(tree)(_ => EmptyTree) ?
+      else tree
     else EmptyTree
 
   private def ifaceTemplate(templ: Template): Template =
@@ -284,20 +297,22 @@ abstract class AddInterfaces extends InfoTransform { self: Erasure =>
     if (treeInfo.firstConstructor(stats) != EmptyTree) stats
     else DefDef(clazz.primaryConstructor, Block(List(), Literal(Constant(())))) :: stats
 
-  private def implTemplate(clazz: Symbol, templ: Template): Template = atPos(templ.pos) {
-    val templ1 = (
-      Template(templ.parents, noSelfType, addMixinConstructorDef(clazz, templ.body map deriveMemberForImplClass))
-        setSymbol clazz.newLocalDummy(templ.pos)
-    )
-    templ1.changeOwner(templ.symbol.owner -> clazz, templ.symbol -> templ1.symbol)
-    templ1
+  private def implTemplate(clazz: Symbol, iface: ClassDef): Template = atPos(iface.impl.pos) {
+    val templ     = iface.impl
+    val exprOwner = clazz.newLocalDummy(templ.pos)
+    val derivedImplClassMembers = templ.body map deriveMemberForImplClass(clazz, iface.symbol, exprOwner)
+    val templWithMixedinMembers = Template(templ.parents, noSelfType, addMixinConstructorDef(clazz, derivedImplClassMembers))
+
+    templWithMixedinMembers setSymbol exprOwner
+    templWithMixedinMembers changeOwner (templ.symbol.owner -> clazz, templ.symbol -> exprOwner)
+    templWithMixedinMembers
   }
 
   def implClassDefs(trees: List[Tree]): List[Tree] = {
     trees collect {
       case cd: ClassDef if cd.symbol.needsImplClass =>
         val clazz = implClass(cd.symbol).initialize
-        ClassDef(clazz, implTemplate(clazz, cd.impl))
+        ClassDef(clazz, implTemplate(clazz, cd))
     }
   }
 

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

@@ -359,7 +359,7 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
       }
 
       log("merging: " + originalStats.mkString("\n") + "\nwith\n" + specializedStats.mkString("\n"))
-      val res = for (s <- originalStats; stat = s.duplicate) yield {
+      for (s <- originalStats; stat = s.duplicate) yield {
         log("merge: looking at " + stat)
         val stat1 = stat match {
           case Assign(sel @ Select(This(_), field), _) =>
@@ -389,9 +389,8 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
         } else
           stat1
       }
-      if (specBuf.nonEmpty)
-        println("residual specialized constructor statements: " + specBuf)
-      res
+//      if (specBuf.nonEmpty)
+//        println("residual specialized constructor statements: " + specBuf)
     }
 
     /* Add an 'if' around the statements coming after the super constructor. This
@@ -548,7 +547,9 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
     // Create an assignment to class field `to` with rhs `from`
     def mkAssign(to: Symbol, from: Tree): Tree =
       localTyper.typedPos(to.pos) {
-        Assign(Select(This(clazz), to), from)
+        val qual = Select(This(clazz), to)
+        if (to.isSetter) Apply(qual, List(from))
+        else Assign(qual, from)
       }
 
     // Create code to copy parameter to parameter accessor field.
@@ -607,9 +608,9 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
         // 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.
         // Returns `true` when a `ValDef` is needed.
-        def moveEffectToCtor(mods: Modifiers, rhs: Tree, memoized: Boolean): Unit = {
+        def moveEffectToCtor(mods: Modifiers, rhs: Tree, assignSym: Symbol): Unit = {
           val initializingRhs =
-            if (!memoized || statSym.isLazy) EmptyTree // not memoized, or effect delayed (for lazy val)
+            if ((assignSym eq NoSymbol) || statSym.isLazy) EmptyTree // not memoized, or effect delayed (for lazy val)
             else if (!mods.hasStaticFlag) intoConstructor(statSym, primaryConstr.symbol)(rhs)
             else rhs
 
@@ -619,7 +620,7 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
               else if (mods hasFlag PRESUPER | PARAMACCESSOR) constrPrefixBuf
               else constrStatBuf
 
-            initPhase += mkAssign(statSym, initializingRhs)
+            initPhase += mkAssign(assignSym, initializingRhs)
           }
         }
 
@@ -632,17 +633,19 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
           // 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 stat: 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:
           //   - the class initializer (static),
           //   - the constructor, before the super call (early initialized or a parameter accessor),
           //   - the constructor, after the super call (regular val).
           case ValDef(mods, _, _, rhs) =>
-            if (statSym.isLazy) {
+            if (rhs ne EmptyTree) {
               val emitField = memoizeValue(statSym)
-              moveEffectToCtor(mods, rhs, emitField)
+              moveEffectToCtor(mods, rhs, statSym)
               if (emitField) defBuf += deriveValDef(stat)(_ => EmptyTree)
             } else defBuf += stat
 

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

@@ -137,14 +137,10 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
     // not stored, no side-effect
     val pureConstant = tp.isInstanceOf[ConstantType]
 
-    val stored = !(pureConstant) // || isUnitType(tp))
-
-
-    //    println(s"fieldMemoizationIn $sym $site = $tp")
-    def assignSym: Symbol =
-      if (!stored) NoSymbol
-      else if (accessorOrField hasFlag ACCESSOR) accessorOrField.setterIn(site)
-      else accessorOrField
+    // if !stored, may still have a side-effect
+    // (currently not distinguished -- used to think we could drop unit-typed vals,
+    //  but the memory model cares about writes to unit-typed fields)
+    val stored = !pureConstant  // || isUnitType(tp))
   }
 
   private def fieldTypeForGetterIn(getter: Symbol, pre: Type): Type = getter.info.finalResultType.asSeenFrom(pre, getter.owner)
@@ -262,7 +258,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
           afterOwnPhase { mixin.info }.decls.toList.filter(accessorImplementedInSubclass)
         }
 
-//        println(s"mixing in $accessorsMaybeNeedingImpl from ${clazz.mixinClasses}")
+//        println(s"mixing in for $clazz: $accessorsMaybeNeedingImpl from ${clazz.mixinClasses}")
 
         // TODO: setter conflicts?
         def accessorConflictsExistingVal(accessor: Symbol): Boolean = {
@@ -289,7 +285,6 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
           // a trait setter for an overridden val will receive a unit body in the tree transform
           if (nme.isTraitSetterName(accessor.name)) {
             val getter = accessor.getterIn(accessor.owner)
-//            println(s"mixing in trait setter ${accessor.defString}: $overridden")
             val clone = cloneAccessor()
 
             clone filterAnnotations (ai => !ai.matches(TraitSetterAnnotationClass)) // only supposed to be set in trait
@@ -372,7 +367,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
         else fieldAccess(getter)
       }
 
-//      println(s"accessorsAndFieldsNeedingTrees: $accessorsAndFieldsNeedingTrees")
+//      println(s"accessorsAndFieldsNeedingTrees for $templateSym: $accessorsAndFieldsNeedingTrees")
       def setterBody(setter: Symbol): Tree = {
         // trait setter in trait
         if (clazz.isTrait) EmptyTree
@@ -393,48 +388,11 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
     def rhsAtOwner(stat: ValOrDefDef, newOwner: Symbol): Tree =
       atOwner(newOwner)(super.transform(stat.rhs.changeOwner(stat.symbol -> newOwner)))
 
-    private def deriveStoredAccessor(clazz: Symbol, stat: ValOrDefDef, fieldMemoization: FieldMemoization, exprOwner: Symbol): List[Tree] = {
-      // Which symbol should own `rhs`, as the expression is lifted from `statSym` to the class's template?
-      // Normally, the local dummy (`exprOwner`) is the owner for stats in the template (not `clazz`!)
-      // when we get owners wrong, the following fails:
-      //   - stackoverflow in uncurry when bootstrapping
-      //   - compiling scala.util.Properties,
-      //     where some references from the impl class are still pointing to the trait interface, not the impl class
-      //   - test/files/trait-defaults/ultimate-nesting.scala,
-      //     where a nested module class is separated from its module, as well as from the `settings` reference
-      //     (this was due to changeOwner not changing a module's moduleClass's owner in addition to the module's owner)
-      val initEffect = {
-        // println(s"initEffect: owner $statSym --> $exprOwner (currentOwner= $currentOwner)")
-        // println(s"initEffect for $assignSym is $rhs")
-
-        val lifted = rhsAtOwner(stat, exprOwner)
-        import fieldMemoization.assignSym
-
-        if (assignSym eq NoSymbol) lifted
-        else localTyper.typedPos(stat.pos) {
-          val qual = Select(This(clazz), assignSym)
-          if (assignSym.isSetter) Apply(qual, List(lifted))
-          else Assign(qual, lifted)
-        }
-      }
-
-      // DefDef(computeSym, rhsAtOwner(stat, computeSym))
-      // def deriveRhs(oldRhs: Tree) = localTyper.typedPos(oldRhs.pos)(Apply(gen.mkAttributedRef(computeSym), Nil))
-
-      def deriveRhs(oldRhs: Tree) = EmptyTree
-      val derivedStat = stat match {
-        case dd: DefDef => deriveDefDef(stat)(deriveRhs)
-        case vd: ValDef => deriveValDef(stat)(deriveRhs)
-      }
-
-      derivedStat :: initEffect :: Nil
-    }
-
     private def transformStat(exprOwner: Symbol)(stat: Tree): List[Tree] = {
       val clazz = currentOwner
       val statSym = stat.symbol
 
-      // println(s"transformStat $statSym in ${exprOwner.ownerChain}")
+//       println(s"transformStat $statSym in ${exprOwner.ownerChain}")
       //        currentRun.trackerFactory.snapshot()
 
       /*
@@ -452,29 +410,38 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
        */
       stat match {
         // TODO: consolidate with ValDef case
-        case stat@DefDef(_, _, _, _, _, rhs) if (rhs ne EmptyTree) && (statSym hasFlag ACCESSOR) && !excludedAccessorOrFieldByFlags(statSym) =>
+        case stat@DefDef(_, _, _, _, _, rhs) if (statSym hasFlag ACCESSOR) && !excludedAccessorOrFieldByFlags(statSym) =>
           /* TODO: defer replacing ConstantTyped tree by the corresponding constant until erasure
-             (until then, trees should not be constant-folded -- only their type tracks the resulting constant) */
+             (until then, trees should not be constant-folded -- only their type tracks the resulting constant)
+             TODO: also remove ACCESSOR flag since there won't be an underlying field to access?
+          */
           def statInlinedConstantRhs =
             if (clazz.isTrait) stat // we've already done this for traits.. the asymmetry will be solved by the above todo
             else deriveDefDef(stat)(_ => gen.mkAttributedQualifier(rhs.tpe))
 
-          val fieldMemoization = fieldMemoizationIn(statSym, clazz)
+          if (rhs ne EmptyTree) {
+            val fieldMemoization = fieldMemoizationIn(statSym, clazz)
 
-          // if we decide to have non-stored fields with initialization effects, the stat's RHS should be replaced by unit
-          // if (!fieldMemoization.stored) deriveUnitDef(stat) else stat
+            // if we decide to have non-stored fields with initialization effects, the stat's RHS should be replaced by unit
+            // if (!fieldMemoization.stored) deriveUnitDef(stat) else stat
 
-          if (fieldMemoization.pureConstant)  statInlinedConstantRhs :: Nil
-          else if (clazz.isTrait) deriveStoredAccessor(clazz, stat, fieldMemoization, exprOwner)
-          else stat :: Nil
+            if (fieldMemoization.pureConstant) statInlinedConstantRhs :: Nil
+            else super.transform(stat) :: Nil
+          } else {
+            stat :: Nil
+          }
 
-        case stat@ValDef(mods, _, _, rhs) if (rhs ne EmptyTree) && !excludedAccessorOrFieldByFlags(statSym) =>
-          val fieldMemoization = fieldMemoizationIn(statSym, clazz)
+        case stat@ValDef(mods, _, _, rhs) if !excludedAccessorOrFieldByFlags(statSym) =>
+          if (rhs ne EmptyTree) {
+            val fieldMemoization = fieldMemoizationIn(statSym, clazz)
 
-          // drop the val for (a) constant (pure & not-stored) and (b) not-stored (but still effectful) fields
-          if (fieldMemoization.pureConstant) Nil // (a)
-          else if (fieldMemoization.stored) deriveStoredAccessor(clazz, stat, fieldMemoization, exprOwner)
-          else rhsAtOwner(stat, exprOwner) :: Nil // (b) -- not used currently
+            // drop the val for (a) constant (pure & not-stored) and (b) not-stored (but still effectful) fields
+            if (fieldMemoization.pureConstant) Nil // (a)
+            else super.transform(stat) :: Nil // if (fieldMemoization.stored)
+//            else rhsAtOwner(transformStat, exprOwner) :: Nil // (b) -- not used currently
+          } else {
+            stat :: Nil
+          }
 
         case tree => List (
           if (exprOwner != currentOwner && tree.isTerm) atOwner(exprOwner)(super.transform(tree))

src/reflect/scala/reflect/internal/Types.scala

@@ -2496,6 +2496,8 @@ trait Types
     override def isJava = true
   }
 
+  // TODO: rename so it's more appropriate for the type that is for a method without argument lists
+  // ("nullary" erroneously implies it has an argument list with zero arguments, it actually has zero argument lists)
   case class NullaryMethodType(override val resultType: Type) extends Type with NullaryMethodTypeApi {
     override def isTrivial = resultType.isTrivial && (resultType eq resultType.withoutAnnotations)
     override def prefix: Type = resultType.prefix