Figuring it couldn't hurt if more people had a ...

Figuring it couldn't hurt if more people had a command of some of
our binary compatibility impacting code, I went over the ModuleDef
elimination with my clarify stick and made the machinery more
transparent, to me anyway. Review by plocinic.

5 files changed, 151 insertions, 149 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
index 7c19d0239e..9d14292981 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
@@ -117,6 +117,11 @@ abstract class TreeGen {
     if (sym.owner.isClass) mkAttributedRef(sym.owner.thisType, sym)
     else mkAttributedIdent(sym)
 
+  /** Builds an untyped reference to given symbol. */
+  def mkUnattributedRef(sym: Symbol): Tree =
+    if (sym.owner.isClass) Select(This(sym.owner), sym)
+    else Ident(sym)
+
   /** Replaces tree type with a stable type if possible */
   def stabilize(tree: Tree): Tree = {
     for(tp <- stableTypeFor(tree)) tree.tpe = tp
@@ -249,7 +254,7 @@ abstract class TreeGen {
   def mkSoftRef(expr: Tree): Tree = New(TypeTree(SoftReferenceClass.tpe), List(List(expr)))
 
   def mkCached(cvar: Symbol, expr: Tree): Tree = {
-    val cvarRef = if (cvar.owner.isClass) Select(This(cvar.owner), cvar) else Ident(cvar)
+    val cvarRef = mkUnattributedRef(cvar)
     Block(
       List(
         If(Apply(Select(cvarRef, nme.eq), List(Literal(Constant(null)))),
@@ -259,17 +264,24 @@ abstract class TreeGen {
     )
   }
 
+  // Builds a tree of the form "{ lhs = rhs ; lhs  }"
+  def mkAssignAndReturn(lhs: Symbol, rhs: Tree): Tree = {
+    val lhsRef = mkAttributedRef(lhs)
+    Block(Assign(lhsRef, rhs) :: Nil, lhsRef)
+  }
+
   def mkModuleVarDef(accessor: Symbol) = {
-    val mval = accessor.owner.newVariable(accessor.pos.focus, nme.moduleVarName(accessor.name))
-      .setInfo(accessor.tpe.finalResultType)
-      .setFlag(LAZY)
-      .setFlag(MODULEVAR)
-    mval.setLazyAccessor(accessor)
+    val mval = (
+      accessor.owner.newVariable(accessor.pos.focus, nme.moduleVarName(accessor.name))
+      setInfo accessor.tpe.finalResultType
+      setFlag (LAZY | MODULEVAR)
+      setLazyAccessor accessor
+    )
     if (mval.owner.isClass) {
       mval setFlag (PRIVATE | LOCAL | SYNTHETIC)
       mval.owner.info.decls.enter(mval)
     }
-    ValDef(mval, EmptyTree)
+    ValDef(mval)
   }
 
   // def m: T = { if (m$ eq null) m$ = new m$class(...) m$ }
diff --git a/src/compiler/scala/tools/nsc/ast/Trees.scala b/src/compiler/scala/tools/nsc/ast/Trees.scala
index dbe4a587ba..0a107990dc 100644
--- a/src/compiler/scala/tools/nsc/ast/Trees.scala
+++ b/src/compiler/scala/tools/nsc/ast/Trees.scala
@@ -76,6 +76,12 @@ trait Trees extends reflect.generic.Trees { self: SymbolTable =>
       ft.result
     }
 
+    def changeOwner(pairs: (Symbol, Symbol)*): Tree = {
+      pairs.foldLeft(tree) { case (t, (oldOwner, newOwner)) =>
+        new ChangeOwnerTraverser(oldOwner, newOwner) apply t
+      }
+    }
+
     /** Is there part of this tree which satisfies predicate `p'? */
     def exists(p: Tree => Boolean): Boolean = !find(p).isEmpty
 
@@ -996,13 +1002,6 @@ trait Trees extends reflect.generic.Trees { self: SymbolTable =>
     }
   }
 
-  final class TreeList {
-    private var trees = List[Tree]()
-    def append(t: Tree): TreeList = { trees = t :: trees; this }
-    def append(ts: List[Tree]): TreeList = { trees = ts reverse_::: trees; this }
-    def toList: List[Tree] = trees.reverse
-  }
-
   object posAssigner extends Traverser {
     var pos: Position = _
     override def traverse(t: Tree) {
diff --git a/src/compiler/scala/tools/nsc/symtab/Symbols.scala b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
index bba07794e7..7c8b2eb2ed 100644
--- a/src/compiler/scala/tools/nsc/symtab/Symbols.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
@@ -1171,6 +1171,9 @@ trait Symbols extends reflect.generic.Symbols { self: SymbolTable =>
     /** For a lazy value, its lazy accessor. NoSymbol for all others */
     def lazyAccessor: Symbol = NoSymbol
 
+    /** If this is a lazy value, the lazy accessor; otherwise this symbol. */
+    def lazyAccessorOrSelf: Symbol = if (isLazy) lazyAccessor else this
+
     /** For an outer accessor: The class from which the outer originates.
      *  For all other symbols: NoSymbol
      */
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index 0795faf6b4..512339e62b 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -819,10 +819,11 @@ abstract class RefChecks extends InfoTransform {
         index = index + 1;
         stat match {
           case ClassDef(_, _, _, _) | DefDef(_, _, _, _, _, _) | ModuleDef(_, _, _) | ValDef(_, _, _, _) =>
-            assert(stat.symbol != NoSymbol, stat);//debug
-            if (stat.symbol.isLocal) {
-              currentLevel.scope.enter(stat.symbol)
-              symIndex(stat.symbol) = index;
+            //assert(stat.symbol != NoSymbol, stat);//debug
+            val sym = stat.symbol.lazyAccessorOrSelf
+            if (sym.isLocal) {
+              currentLevel.scope.enter(sym)
+              symIndex(sym) = index;
             }
           case _ =>
         }
@@ -915,11 +916,75 @@ abstract class RefChecks extends InfoTransform {
 
     override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = {
       pushLevel()
-      enterSyms(stats)
-      var index = -1
-      val stats1 = stats flatMap { stat => index += 1; transformStat(stat, index) }
-      popLevel()
-      stats1
+      try {
+        enterSyms(stats)
+        var index = -1
+        stats flatMap { stat => index += 1; transformStat(stat, index) }
+      }
+      finally popLevel()
+    }
+
+    /** Eliminate ModuleDefs.
+     *   - A top level object is replaced with their module class.
+     *   - An inner object is transformed into a module var, created on first access.
+     *
+     *  In both cases, this transformation returns the list of replacement trees:
+     *   - Top level: the module class accessor definition
+     *   - Inner: a class definition, declaration of module var, and module var accessor
+     */
+    private def eliminateModuleDefs(tree: Tree): List[Tree] = {
+      val ModuleDef(mods, name, impl) = tree
+      val sym = tree.symbol
+
+      // transformedInfo check is necessary here because the object info may already
+      // have been transformed, and we do not want to have duplicate lazy accessors
+      // (through duplicate nested object -> lazy val transformation.)
+      val transformedInfo = sym.isLazy
+      val classSym        = if (transformedInfo) sym.lazyAccessor else sym.moduleClass
+      val cdef            = ClassDef(mods | MODULE, name.toTypeName, Nil, impl) setSymbol classSym setType NoType
+
+      def findOrCreateModuleVar() = localTyper.typedPos(tree.pos) {
+        lazy val createModuleVar = gen.mkModuleVarDef(sym)
+        if (!transformedInfo) createModuleVar
+        else sym.owner.info.decl(nme.moduleVarName(sym.name.toTermName)) match {
+          // In case we are dealing with local symbol then we already have
+          // to correct error with forward reference
+          case NoSymbol => createModuleVar
+          case vsym     => ValDef(vsym)
+        }
+      }
+      def createStaticModuleAccessor() = atPhase(phase.next) {
+        val method = (
+          sym.owner.newMethod(sym.pos, sym.name.toTermName)
+          setFlag (sym.flags | STABLE) resetFlag MODULE setInfo NullaryMethodType(sym.moduleClass.tpe)
+        )
+        sym.owner.info.decls enter method
+        localTyper.typedPos(tree.pos)(gen.mkModuleAccessDef(method, sym))
+      }
+      def createInnerModuleAccessor(vdef: Tree) = List(
+        vdef,
+        localTyper.typedPos(tree.pos) {
+          val vsym = vdef.symbol
+          atPhase(phase.next) {
+            val rhs  = gen.newModule(sym, vsym.tpe)
+            // side effecting symbol flags
+            if (!transformedInfo) {
+              sym resetFlag (MODULE | FINAL | CASE)
+              sym setFlag (LAZY | ACCESSOR | SYNTHETIC)
+              sym setInfo NullaryMethodType(sym.tpe)
+              sym setFlag (lateMETHOD | STABLE)
+            }
+            val body = if (sym.owner.isTrait) rhs else gen.mkAssignAndReturn(vsym, rhs)
+            DefDef(sym, body.changeOwner(vsym -> sym))
+          }
+        }
+      )
+      transformTrees(cdef :: {
+        if (sym.isStatic)
+          if (sym.allOverriddenSymbols.isEmpty) Nil
+          else List(createStaticModuleAccessor())
+        else createInnerModuleAccessor(findOrCreateModuleVar)
+      })
     }
 
     /** Implements lazy value accessors:
@@ -928,120 +993,47 @@ abstract class RefChecks extends InfoTransform {
      *    - for all other lazy values z the accessor is a block of this form:
      *      { z = <rhs>; z } where z can be an identifier or a field.
      */
-    def transformStat(tree: Tree, index: Int): List[Tree] = tree match {
-      case ModuleDef(mods, name, impl) =>
-        val sym = tree.symbol
-        if (sym.isStatic) {
-          val cdef = ClassDef(mods | MODULE, name, List(), impl)
-            .setPos(tree.pos)
-            .setSymbol(sym.moduleClass)
-            .setType(NoType)
-
-          if (!sym.allOverriddenSymbols.isEmpty) {
-            val factory = sym.owner.newMethod(sym.pos, sym.name)
-              .setFlag(sym.flags | STABLE).resetFlag(MODULE)
-              .setInfo(PolyType(List(), sym.moduleClass.tpe))
-            sym.owner.info.decls.enter(factory)
-            val ddef =
-              atPhase(phase.next) {
-                localTyper.typed {
-                  gen.mkModuleAccessDef(factory, sym.tpe)
-                }
-              }
-            transformTrees(List(cdef, ddef))
-          } else {
-            List(transform(cdef))
-          }
-        } else {
-          def lazyNestedObjectTrees(transformedInfo: Boolean) = {
-            val cdef = ClassDef(mods | MODULE, name, List(), impl)
-              .setPos(tree.pos)
-              .setSymbol(if (!transformedInfo) sym.moduleClass else sym.lazyAccessor)
-              .setType(NoType)
-
-            val vdef = localTyper.typedPos(tree.pos){
-              if (!transformedInfo)
-                gen.mkModuleVarDef(sym)
-              else {
-                val vsym = sym.owner.info.decl(nme.moduleVarName(sym.name))
-                assert(vsym != NoSymbol, "Nested object after transformInfo set module variable")
-                ValDef(vsym, EmptyTree)
-              }
-            }
-            val vsym = vdef.symbol
-
-            val ddef = atPhase(phase.next) {
-              localTyper.typed {
-                val rhs = gen.newModule(sym, vsym.tpe)
-                if (!transformedInfo) {
-                  sym.resetFlag(MODULE | FINAL | CASE)
-                  sym.setFlag(LAZY | ACCESSOR | SYNTHETIC)
-
-                  sym.setInfo(PolyType(List(), sym.tpe))
-                  sym setFlag (lateMETHOD | STABLE)
-                }
+    private def makeLazyAccessor(tree: Tree, rhs: Tree): List[Tree] = {
+      val vsym        = tree.symbol
+      assert(vsym.isTerm, vsym)
+      val hasUnitType = vsym.tpe.typeSymbol == UnitClass
+      val lazySym     = vsym.lazyAccessor
+      assert(lazySym != NoSymbol, vsym)
+
+      // for traits, this is further transformed in mixins
+      val body = (
+        if (tree.symbol.owner.isTrait || hasUnitType) rhs
+        else gen.mkAssignAndReturn(vsym, rhs)
+      )
+      val lazyDef = atPos(tree.pos)(DefDef(lazySym, body.changeOwner(vsym -> lazySym)))
+      log("Made lazy def: " + lazyDef)
 
-                val ownerTransformer = new ChangeOwnerTraverser(vsym, sym)
-                val lazyDef = atPos(tree.pos)(
-                  DefDef(sym, ownerTransformer(
-                    if (sym.owner.isTrait) rhs
-                    else Block(List(
-                            Assign(gen.mkAttributedRef(vsym), rhs)),
-                            gen.mkAttributedRef(vsym)))
-                       ))
-                lazyDef
-              }
-            }
-            transformTrees(List(cdef, vdef, ddef))
-          }
-          lazyNestedObjectTrees(sym.hasFlag(LAZY))
-        }
+      if (hasUnitType) List(typed(lazyDef))
+      else List(
+        typed(ValDef(vsym)),
+        atPhase(phase.next)(typed(lazyDef))
+      )
+    }
 
+    def transformStat(tree: Tree, index: Int): List[Tree] = tree match {
+      case ModuleDef(_, _, _) => eliminateModuleDefs(tree)
       case ValDef(_, _, _, _) =>
-        val tree1 = transform(tree); // important to do before forward reference check
-        val ValDef(_, _, _, rhs) = tree1
-        if (tree.symbol.hasFlag(LAZY)) {
-          assert(tree.symbol.isTerm, tree.symbol)
-          val vsym = tree.symbol
-          val hasUnitType = (tree.symbol.tpe.typeSymbol == UnitClass)
-          val lazyDefSym = vsym.lazyAccessor
-          assert(lazyDefSym != NoSymbol, vsym)
-          val ownerTransformer = new ChangeOwnerTraverser(vsym, lazyDefSym)
-          val lazyDef = atPos(tree.pos)(
-              DefDef(lazyDefSym, ownerTransformer(
-                if (tree.symbol.owner.isTrait // for traits, this is further transformed in mixins
-                    || hasUnitType) rhs
-                else Block(List(
-                       Assign(gen.mkAttributedRef(vsym), rhs)),
-                       gen.mkAttributedRef(vsym)))))
-          log("Made lazy def: " + lazyDef)
-          if (hasUnitType)
-            typed(lazyDef) :: Nil
-          else
-            typed(ValDef(vsym, EmptyTree)) :: atPhase(phase.next) { typed(lazyDef) } :: Nil
-        } else {
-          if (tree.symbol.isLocal && index <= currentLevel.maxindex) {
+        val tree1 @ ValDef(_, _, _, rhs) = transform(tree) // important to do before forward reference check
+        if (tree.symbol.isLazy)
+          makeLazyAccessor(tree, rhs)
+        else {
+          val lazySym = tree.symbol.lazyAccessorOrSelf
+          if (lazySym.isLocal && index <= currentLevel.maxindex) {
             if (settings.debug.value)
               Console.println(currentLevel.refsym)
-            unit.error(currentLevel.refpos, "forward reference extends over definition of " + tree.symbol);
+            unit.error(currentLevel.refpos, "forward reference extends over definition of " + lazySym)
           }
           List(tree1)
         }
-
-      case Import(_, _) =>
-        List()
-
-      case _ =>
-        List(transform(tree))
+      case Import(_, _) => Nil
+      case _            => List(transform(tree))
     }
 
-    /******** Begin transform inner function section ********/
-
-    /** The private functions between here and 'transform' are conceptually
-     *  inner functions to that method, but have been moved outside of it to
-     *  ease the burden on the optimizer.
-     */
-
     /* Check whether argument types conform to bounds of type parameters */
     private def checkBounds(pre: Type, owner: Symbol, tparams: List[Symbol], argtps: List[Type], pos: Position): Unit =
       try typer.infer.checkBounds(pos, pre, owner, tparams, argtps, "")
@@ -1055,27 +1047,24 @@ abstract class RefChecks extends InfoTransform {
             ()
           }
       }
-    private def isIrrefutable(pat: Tree, seltpe: Type): Boolean = {
-      val result = pat match {
-        case Apply(_, args) =>
-          val clazz = pat.tpe.typeSymbol;
-          clazz == seltpe.typeSymbol &&
-          clazz.isClass && (clazz hasFlag CASE) &&
-          (args corresponds clazz.primaryConstructor.tpe.asSeenFrom(seltpe, clazz).paramTypes)(isIrrefutable) // @PP: corresponds
-        case Typed(pat, tpt) =>
-          seltpe <:< tpt.tpe
-        case Ident(nme.WILDCARD) =>
-          true
-        case Bind(_, pat) =>
-          isIrrefutable(pat, seltpe)
-        case _ =>
-          false
-      }
-      //Console.println("is irefutable? " + pat + ":" + pat.tpe + " against " + seltpe + ": " + result);//DEBUG
-      result
+    private def isIrrefutable(pat: Tree, seltpe: Type): Boolean = pat match {
+      case Apply(_, args) =>
+        val clazz = pat.tpe.typeSymbol
+        clazz == seltpe.typeSymbol &&
+        clazz.isCaseClass &&
+        (args corresponds clazz.primaryConstructor.tpe.asSeenFrom(seltpe, clazz).paramTypes)(isIrrefutable)
+      case Typed(pat, tpt) =>
+        seltpe <:< tpt.tpe
+      case Ident(tpnme.WILDCARD) =>
+        true
+      case Bind(_, pat) =>
+        isIrrefutable(pat, seltpe)
+      case _ =>
+        false
     }
-    /** If symbol is deprecated and is not contained in a deprecated definition,
-     *  issue a deprecated warning
+
+    /** If symbol is deprecated, and the point of reference is not enclosed
+     *  in either a deprecated member or a scala bridge method, issue a warning.
      */
     private def checkDeprecated(sym: Symbol, pos: Position) {
       if (sym.isDeprecated && !currentOwner.ownerChain.exists(_.isDeprecated)) {
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index 3b2c5263e3..65e752d544 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -3239,8 +3239,7 @@ trait Typers { self: Analyzer =>
           context.tree match {
             case ValDef(mods, _, _, Apply(Select(`tree`, _), _)) if !(mods hasFlag MUTABLE) && sym != null && sym != NoSymbol =>
               val sym1 = if (sym.owner.isClass && sym.getter(sym.owner) != NoSymbol) sym.getter(sym.owner)
-                else if ((sym hasFlag LAZY) && sym.lazyAccessor != NoSymbol) sym.lazyAccessor
-                else sym
+                else sym.lazyAccessorOrSelf
               val pre = if (sym1.owner.isClass) sym1.owner.thisType else NoPrefix
               intersectionType(List(tp, singleType(pre, sym1)))
             case _ => tp