Overhauling mixin.

Feel the bugs falling away.

1 files changed, 140 insertions, 146 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index 639e060812..adbb7d43d0 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -24,7 +24,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
   /** This map contains a binding (class -> info) if
    *  the class with this info at phase mixinPhase has been treated for mixin composition
    */
-  private val treatedClassInfos = perRunCaches.newMap[Symbol, Type]()
+  private val treatedClassInfos = perRunCaches.newMap[Symbol, Type]() withDefaultValue NoType
 
   /** Map a lazy, mixedin field accessor to it's trait member accessor */
   private val initializer = perRunCaches.newMap[Symbol, Symbol]
@@ -104,7 +104,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
    */
   private val toInterfaceMap = new TypeMap {
     def apply(tp: Type): Type = mapOver( tp match {
-      case TypeRef(pre, sym, args) if (sym.isImplClass) =>
+      case TypeRef(pre, sym, args) if sym.isImplClass =>
         typeRef(pre, beforeMixin(sym.toInterface), args)
       case _ => tp
     })
@@ -113,10 +113,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
   /** The implementation class corresponding to a currently compiled interface.
    *  todo: try to use Symbol.implClass instead?
    */
-  private def implClass(iface: Symbol): Symbol = {
-    val impl = iface.implClass
-    if (impl != NoSymbol) impl else erasure.implClass(iface)
-  }
+  private def implClass(iface: Symbol) = iface.implClass orElse (erasure implClass iface)
 
   /** Returns the symbol that is accessed by a super-accessor in a mixin composition.
    *
@@ -168,6 +165,29 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
     debuglog("new member of " + clazz + ":" + member.defString)
     clazz.info.decls enter member setFlag MIXEDIN
   }
+  def cloneAndAddMember(mixinClass: Symbol, mixinMember: Symbol, clazz: Symbol): Symbol =
+    addMember(clazz, cloneBeforeErasure(mixinClass, mixinMember, clazz))
+
+  def cloneBeforeErasure(mixinClass: Symbol, mixinMember: Symbol, clazz: Symbol): Symbol = {
+    val newSym = beforeErasure {
+      // since we used `mixinMember` from the interface that represents the trait that's
+      // being mixed in, have to instantiate the interface type params (that may occur in mixinMember's
+      // info) as they are seen from the class.  We can't use the member that we get from the
+      // implementation class, as it's a clone that was made after erasure, and thus it does not
+      // know its info at the beginning of erasure anymore.
+      //   Optimize: no need if mixinClass has no typeparams.
+      mixinMember cloneSymbol clazz modifyInfo (info =>
+        if (mixinClass.typeParams.isEmpty) info
+        else (clazz.thisType baseType mixinClass) memberInfo mixinMember
+      )
+    }
+    // clone before erasure got rid of type info we'll need to generate a javaSig
+    // now we'll have the type info at (the beginning of) erasure in our history,
+    // and now newSym has the info that's been transformed to fit this period
+    // (no need for asSeenFrom as phase.erasedTypes)
+    // TODO: verify we need the updateInfo and document why
+    newSym updateInfo (mixinMember.info cloneInfo newSym)
+  }
 
   def needsExpandedSetterName(field: Symbol) = !field.isLazy && (
     if (field.isMethod) field.hasStableFlag
@@ -181,9 +201,12 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
    *    - lazy fields don't get a setter.
    */
   def addLateInterfaceMembers(clazz: Symbol) {
-    if ((treatedClassInfos get clazz) != Some(clazz.info)) {
+    def makeConcrete(member: Symbol) =
+      member setPos clazz.pos resetFlag (DEFERRED | lateDEFERRED)
+
+    if (treatedClassInfos(clazz) != clazz.info) {
       treatedClassInfos(clazz) = clazz.info
-      assert(phase == currentRun.mixinPhase)
+      assert(phase == currentRun.mixinPhase, phase)
 
       /** Create a new getter. Getters are never private or local. They are
        *  always accessors and deferred. */
@@ -210,8 +233,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
       }
 
       clazz.info // make sure info is up to date, so that implClass is set.
-      val impl = implClass(clazz)
-      assert(impl != NoSymbol)
+      val impl = implClass(clazz) orElse abort("No impl class for " + clazz)
 
       for (member <- impl.info.decls) {
         if (!member.isMethod && !member.isModule && !member.isModuleVar) {
@@ -242,139 +264,107 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
    *      - for every super accessor in T, add an implementation of that accessor
    *      - for every module in T, add a module
    */
-  def addMixedinMembers(clazz: Symbol, unit : CompilationUnit) {
-    def cloneBeforeErasure(iface: Symbol, clazz: Symbol, imember: Symbol): Symbol = {
-      val newSym = beforeErasure {
-        val res = imember.cloneSymbol(clazz)
-        // since we used the member (imember) from the interface that represents the trait that's being mixed in,
-        // have to instantiate the interface type params (that may occur in imember's info) as they are seen from the class
-        // we can't use the member that we get from the implementation class, as it's a clone that was made after erasure,
-        // and thus it does not know its info at the beginning of erasure anymore
-        // optimize: no need if iface has no typeparams
-        if(iface.typeParams nonEmpty) res.setInfo(clazz.thisType.baseType(iface).memberInfo(imember))
-        res
-      } // clone before erasure got rid of type info we'll need to generate a javaSig
-      // now we'll have the type info at (the beginning of) erasure in our history,
-      newSym.updateInfo(imember.info.cloneInfo(newSym)) // and now newSym has the info that's been transformed to fit this period (no need for asSeenFrom as phase.erasedTypes)
-      newSym // TODO: verify we need the updateInfo and document why
-    }
-
-    if (!(clazz hasFlag JAVA) && (treatedClassInfos get clazz) != Some(clazz.info)) {
-      treatedClassInfos(clazz) = clazz.info
-
-      assert(!clazz.isTrait, clazz)
-      assert(clazz.info.parents.nonEmpty, clazz)
-
-      // first complete the superclass with mixed in members
-      addMixedinMembers(clazz.superClass, unit)
-
-      //Console.println("adding members of " + clazz.info.baseClasses.tail.takeWhile(superclazz !=) + " to " + clazz);//DEBUG
-
-      /** Mix in members of implementation class mixinClass into class clazz */
-      def mixinImplClassMembers(impl: Symbol, iface: Symbol) {
-        assert(
-          // XXX this should be impl.isImplClass, except that we get impl classes
-          // coming through under -optimise which do not agree that they are (because
-          // the IMPLCLASS flag is unset, I believe.) See ticket #4285.
-          nme.isImplClassName(impl.name) || impl.isImplClass,
-          "%s (%s) is not a an implementation class, it cannot mix in %s".format(
-            impl, impl.defaultFlagString, iface)
-        )
-        if (!impl.isImplClass) {
-          debugwarn("!!! " + impl + " has an impl class name, but !isImplClass: " + impl.defaultFlagString + ", mixing in " + iface)
-        }
+  def addMixedinMembers(clazz: Symbol, unit: CompilationUnit) {
+    def cloneAndAddMixinMember(mixinClass: Symbol, mixinMember: Symbol): Symbol = (
+      cloneAndAddMember(mixinClass, mixinMember, clazz)
+           setPos clazz.pos
+        resetFlag DEFERRED | lateDEFERRED
+    )
 
-        for (member <- impl.info.decls) {
-          if (isForwarded(member)) {
-            val imember = member.overriddenSymbol(iface)
-            // atPhase(currentRun.erasurePhase){
-            //   println(""+(clazz, iface, clazz.typeParams, iface.typeParams, imember, clazz.thisType.baseType(iface), clazz.thisType.baseType(iface).memberInfo(imember), imember.info substSym(iface.typeParams, clazz.typeParams)  ))
-            // }
-            // Console.println("mixin member "+member+":"+member.tpe+member.locationString+" "+imember+" "+imember.overridingSymbol(clazz)+" to "+clazz+" with scope "+clazz.info.decls)//DEBUG
-            if (imember.overridingSymbol(clazz) == NoSymbol &&
-                clazz.info.findMember(member.name, 0, lateDEFERRED, false).alternatives.contains(imember)) {
-                  val member1 = addMember(
-                    clazz,
-                    cloneBeforeErasure(iface, clazz, imember) setPos clazz.pos resetFlag (DEFERRED | lateDEFERRED))
-                  member1.asInstanceOf[TermSymbol] setAlias member;
-                }
-          }
+    /** Mix in members of implementation class mixinClass into class clazz */
+    def mixinImplClassMembers(mixinClass: Symbol, mixinInterface: Symbol) {
+      assert(mixinClass.isImplClass, "Not an impl class:" +
+        ((mixinClass.debugLocationString, mixinInterface.debugLocationString)))
+
+      for (member <- mixinClass.info.decls ; if isForwarded(member)) {
+        val imember = member overriddenSymbol mixinInterface
+        imember overridingSymbol clazz match {
+          case NoSymbol =>
+            if (clazz.info.findMember(member.name, 0, lateDEFERRED, false).alternatives contains imember)
+              cloneAndAddMixinMember(mixinInterface, imember).asInstanceOf[TermSymbol] setAlias member
+          case _        =>
         }
       }
+    }
 
-      /** Mix in members of trait mixinClass into class clazz. Also,
-       *  for each lazy field in mixinClass, add a link from its mixed in member to its
-       *  initializer method inside the implclass.
-       */
-      def mixinTraitMembers(mixinClass: Symbol) {
-        // For all members of a trait's interface do:
-        for (member <- mixinClass.info.decls) {
-          if (isConcreteAccessor(member)) {
-            if (isOverriddenAccessor(member, clazz.info.baseClasses)) {
-              debugwarn("!!! is overridden val: "+member.fullLocationString)
+    /** Mix in members of trait mixinClass into class clazz. Also,
+     *  for each lazy field in mixinClass, add a link from its mixed in member to its
+     *  initializer method inside the implclass.
+     */
+    def mixinTraitMembers(mixinClass: Symbol) {
+      // For all members of a trait's interface do:
+      for (mixinMember <- mixinClass.info.decls) {
+        if (isConcreteAccessor(mixinMember)) {
+          if (isOverriddenAccessor(mixinMember, clazz.info.baseClasses))
+            debugwarn("!!! is overridden val: "+mixinMember.fullLocationString)
+          else {
+            // mixin field accessors
+            val mixedInAccessor = cloneAndAddMixinMember(mixinClass, mixinMember)
+            if (mixinMember.isLazy) {
+              initializer(mixedInAccessor) = (
+                implClass(mixinClass).info.decl(mixinMember.name)
+                  orElse abort("Could not find initializer for " + mixinMember.name)
+              )
             }
-            else {
-              // mixin field accessors
-              val member1 = addMember(
-                clazz,
-                cloneBeforeErasure(mixinClass, clazz, member) //member.cloneSymbol(clazz)
-                  setPos clazz.pos
-                  resetFlag (DEFERRED | lateDEFERRED))
-              // println("mixing in: "+ (member, member.info, member1.info))
-              // atPhase(currentRun.erasurePhase){
-              //   println("before erasure: "+ (member.info, member1.info))
-              // }
-              if (member.isLazy) {
-                var init = implClass(mixinClass).info.decl(member.name)
-                assert(init != NoSymbol, "Could not find initializer for " + member.name)
-                initializer(member1) = init
+            if (!mixinMember.isSetter)
+              mixinMember.tpe match {
+                case MethodType(Nil, ConstantType(_)) =>
+                  // mixinMember is a constant; only getter is needed
+                  ;
+                case MethodType(Nil, TypeRef(_, UnitClass, _)) =>
+                  // mixinMember is a value of type unit. No field needed
+                  ;
+                case _ => // otherwise mixin a field as well
+                  // atPhase: the private field is moved to the implementation class by erasure,
+                  // so it can no longer be found in the mixinMember's owner (the trait)
+                  val accessed = beforePickler(mixinMember.accessed)
+                  // #3857, need to retain info before erasure when cloning (since cloning only
+                  // carries over the current entry in the type history)
+                  val sym = beforeErasure {
+                    // so we have a type history entry before erasure
+                    clazz.newValue(nme.getterToLocal(mixinMember.name), mixinMember.pos).setInfo(mixinMember.tpe.resultType)
+                  }
+                  sym updateInfo mixinMember.tpe.resultType // info at current phase
+
+                  val newFlags = (
+                      ( PrivateLocal )
+                    | ( mixinMember getFlag MUTABLE | LAZY)
+                    | ( if (mixinMember.hasStableFlag) 0 else MUTABLE )
+                  )
+
+                  addMember(clazz, sym setFlag newFlags setAnnotations accessed.annotations)
               }
-              if (!member.isSetter)
-                member.tpe match {
-                  case MethodType(Nil, ConstantType(_)) =>
-                    // member is a constant; only getter is needed
-                    ;
-                  case MethodType(Nil, TypeRef(_, UnitClass, _)) =>
-                    // member is a value of type unit. No field needed
-                    ;
-                  case _ => // otherwise mixin a field as well
-                    // atPhase: the private field is moved to the implementation class by erasure,
-                    // so it can no longer be found in the member's owner (the trait)
-                    val accessed = beforePickler(member.accessed)
-                    val sym = beforeErasure { // #3857, need to retain info before erasure when cloning (since cloning only carries over the current entry in the type history)
-                      clazz.newValue(nme.getterToLocal(member.name), member.pos).setInfo(member.tpe.resultType) // so we have a type history entry before erasure
-                    }
-                    sym.updateInfo(member.tpe.resultType) // info at current phase
-                    addMember(clazz,
-                              sym
-                                setFlag (PrivateLocal | member.getFlag(MUTABLE | LAZY))
-                                setFlag (if (!member.hasStableFlag) MUTABLE else 0)
-                                setAnnotations accessed.annotations)
-                }
-            }
-          }
-          else if (member.isSuperAccessor) { // mixin super accessors
-            val member1 = addMember(clazz, member.cloneSymbol(clazz)) setPos clazz.pos
-            assert(member1.alias != NoSymbol, member1)
-            val alias1 = rebindSuper(clazz, member.alias, mixinClass)
-            member1.asInstanceOf[TermSymbol] setAlias alias1
-
-          }
-          else if (member.isMethod && member.isModule && member.hasNoFlags(LIFTED | BRIDGE)) {
-            // mixin objects: todo what happens with abstract objects?
-            addMember(clazz, member.cloneSymbol(clazz, member.flags & ~(DEFERRED | lateDEFERRED)) setPos clazz.pos)
           }
         }
+        else if (mixinMember.isSuperAccessor) { // mixin super accessors
+          val superAccessor = addMember(clazz, mixinMember.cloneSymbol(clazz)) setPos clazz.pos
+          assert(superAccessor.alias != NoSymbol, superAccessor)
+          val alias1 = rebindSuper(clazz, mixinMember.alias, mixinClass)
+          superAccessor.asInstanceOf[TermSymbol] setAlias alias1
+        }
+        else if (mixinMember.isMethod && mixinMember.isModule && mixinMember.hasNoFlags(LIFTED | BRIDGE)) {
+          // mixin objects: todo what happens with abstract objects?
+          addMember(clazz, mixinMember.cloneSymbol(clazz, mixinMember.flags & ~(DEFERRED | lateDEFERRED)) setPos clazz.pos)
+        }
       }
+    }
 
-      for (mc <- clazz.mixinClasses)
-        if (mc hasFlag lateINTERFACE) {
-          // @SEAN: adding trait tracking so we don't have to recompile transitive closures
-          unit.depends += mc
-          addLateInterfaceMembers(mc)
-          mixinTraitMembers(mc)
-          mixinImplClassMembers(implClass(mc), mc)
-        }
+    if (clazz.isJavaDefined || treatedClassInfos(clazz) == clazz.info)
+      return
+
+    treatedClassInfos(clazz) = clazz.info
+    assert(!clazz.isTrait && clazz.info.parents.nonEmpty, clazz)
+
+    // first complete the superclass with mixed in members
+    addMixedinMembers(clazz.superClass, unit)
+
+    //Console.println("adding members of " + clazz.info.baseClasses.tail.takeWhile(superclazz !=) + " to " + clazz);//DEBUG
+    for (mc <- clazz.mixinClasses ; if mc hasFlag lateINTERFACE) {
+      // @SEAN: adding trait tracking so we don't have to recompile transitive closures
+      unit.depends += mc
+      addLateInterfaceMembers(mc)
+      mixinTraitMembers(mc)
+      mixinImplClassMembers(implClass(mc), mc)
     }
   }
 
@@ -436,9 +426,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
    *  Such fields will be nulled after the initializer has memoized the lazy value.
    */
   def singleUseFields(templ: Template): collection.Map[Symbol, List[Symbol]] = {
-    val usedIn = new mutable.HashMap[Symbol, List[Symbol]] {
-      override def default(key: Symbol) = Nil
-    }
+    val usedIn = mutable.HashMap[Symbol, List[Symbol]]() withDefaultValue Nil
 
     object SingleUseTraverser extends Traverser {
       override def traverse(tree: Tree) {
@@ -477,7 +465,6 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
     new MixinTransformer(unit)
 
   class MixinTransformer(unit : CompilationUnit) extends Transformer {
-
     /** Within a static implementation method: the parameter referring to the
      *  current object.  Undefined everywhere else.
      */
@@ -1129,18 +1116,20 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
      *    - refer to fields in some implementation class via an abstract method in the interface.
      */
     private def postTransform(tree: Tree): Tree = {
+      def siteWithinImplClass = currentOwner.enclClass.isImplClass
       val sym = tree.symbol
+
       // change every node type that refers to an implementation class to its
       // corresponding interface, unless the node's symbol is an implementation class.
       if (tree.tpe.typeSymbol.isImplClass && ((sym eq null) || !sym.isImplClass))
         tree.tpe = toInterface(tree.tpe)
 
       tree match {
-        case Template(parents, self, body) =>
+        case templ @ Template(parents, self, body) =>
           // change parents of templates to conform to parents in the symbol info
           val parents1 = currentOwner.info.parents map (t => TypeTree(t) setPos tree.pos)
           // mark fields which can be nulled afterward
-          lazyValNullables = nullableFields(tree.asInstanceOf[Template]) withDefaultValue Set()
+          lazyValNullables = nullableFields(templ) withDefaultValue Set()
           // add all new definitions to current class or interface
           treeCopy.Template(tree, parents1, self, addNewDefs(currentOwner, body))
 
@@ -1179,18 +1168,23 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
               //  - if `m` refers to a trait, insert a static call to the corresponding static
               //    implementation
               //  - otherwise return tree unchanged
-              if (mix == tpnme.EMPTY && currentOwner.enclClass.isImplClass)
-                assert(false, "illegal super in trait: " + currentOwner.enclClass + " " + tree);
+              assert(
+                !(mix == tpnme.EMPTY && siteWithinImplClass),
+                "illegal super in trait: " + currentOwner.enclClass + " " + tree
+              )
+
               if (sym.owner hasFlag lateINTERFACE) {
                 if (sym.hasAccessorFlag) {
                   assert(args.isEmpty, args)
                   val sym1 = sym.overridingSymbol(currentOwner.enclClass)
                   typedPos(tree.pos)((transformSuper(qual) DOT sym1)())
-                } else {
+                }
+                else {
                   staticCall(beforePrevPhase(sym.overridingSymbol(implClass(sym.owner))))
                 }
-              } else {
-                assert(!currentOwner.enclClass.isImplClass, currentOwner.enclClass)
+              }
+              else {
+                assert(!siteWithinImplClass, currentOwner.enclClass)
                 tree
               }
             case _ =>
@@ -1208,8 +1202,8 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
           // refer to fields in some implementation class via an abstract
           // getter in the interface.
           val iface  = toInterface(sym.owner.tpe).typeSymbol
-          val getter = sym.getter(iface)
-          assert(getter != NoSymbol, sym)
+          val getter = sym getter iface orElse abort("No getter for " + sym + " in " + iface)
+
           typedPos(tree.pos)((qual DOT getter)())
 
         case Assign(Apply(lhs @ Select(qual, _), List()), rhs) =>