SI-9946 don't null field in lazy accessors that turn out to be live

If a non-transient lazy val is the only user of a private field in
a class, the field is nulled out at the end of the lazy initializer.
This is tested in the existing test `run/lazy-leaks.scala`.

The analysis of which fields could be nulled out was recently moved
from `mixin` to the new `fields` phase. This introduced a regression
as a reference from an inner- or companion-classes had not yet
been processed by `explicitouter` to publicise private fields.

This commit delays the analysis to mixin (after explicit outer has
done its work.)

Navigating from `foo$lzycompute()` to `foo()` to `foo` is a little
dirty now. I'm not sure whether there is a more robust way to
structure things.

4 files changed, 82 insertions, 73 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala b/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala
index f1ac2287e2..a1923ead21 100644
--- a/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala
+++ b/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala
@@ -326,16 +326,7 @@ trait AccessorSynthesis extends Transform with ast.TreeDSL {
         *
         * This way the inliner should optimize the fast path because the method body is small enough.
         */
-      def expandLazyClassMember(lazyVar: Symbol, lazyAccessor: Symbol, transformedRhs: Tree, nullables: Map[Symbol, List[Symbol]]): Tree = {
-        // use cast so that specialization can turn null.asInstanceOf[T] into null.asInstanceOf[Long]
-        def nullify(sym: Symbol) =
-          Select(thisRef, sym.accessedOrSelf) === gen.mkAsInstanceOf(NULL, sym.info.resultType)
-
-        val nulls = nullables.getOrElse(lazyAccessor, Nil) map nullify
-
-        if (nulls.nonEmpty)
-          log("nulling fields inside " + lazyAccessor + ": " + nulls)
-
+      def expandLazyClassMember(lazyVar: global.Symbol, lazyAccessor: global.Symbol, transformedRhs: global.Tree): Tree = {
         val slowPathSym  = slowPathFor(lazyAccessor)
         val rhsAtSlowDef = transformedRhs.changeOwner(lazyAccessor -> slowPathSym)
 
@@ -346,7 +337,7 @@ trait AccessorSynthesis extends Transform with ast.TreeDSL {
         def needsInit = mkTest(lazyAccessor)
         val doInit = Block(List(storeRes), mkSetFlag(lazyAccessor))
         // the slow part of double-checked locking (TODO: is this the most efficient pattern? https://github.come/scala/scala-dev/issues/204)
-        val slowPathRhs = Block(gen.mkSynchronized(thisRef)(If(needsInit, doInit, EmptyTree)) :: nulls, selectVar)
+        val slowPathRhs = Block(gen.mkSynchronized(thisRef)(If(needsInit, doInit, EmptyTree)) :: Nil, selectVar)
 
         // The lazy accessor delegates to the compute method if needed, otherwise just accesses the var (it was initialized previously)
         // `if ((bitmap&n & MASK) == 0) this.l$compute() else l$`
@@ -358,59 +349,6 @@ trait AccessorSynthesis extends Transform with ast.TreeDSL {
       }
     }
 
-    /** Map lazy values to the fields they should null after initialization. */
-    // TODO: fix
-    def lazyValNullables(clazz: Symbol, templStats: List[Tree]): Map[Symbol, List[Symbol]] = {
-      // if there are no lazy fields, take the fast path and save a traversal of the whole AST
-      if (!clazz.info.decls.exists(_.isLazy)) Map()
-      else {
-        // A map of single-use fields to the lazy value that uses them during initialization.
-        // Each field has to be private and defined in the enclosing class, and there must
-        // be exactly one lazy value using it.
-        //
-        // Such fields will be nulled after the initializer has memoized the lazy value.
-        val singleUseFields: Map[Symbol, List[Symbol]] = {
-          val usedIn = mutable.HashMap[Symbol, List[Symbol]]() withDefaultValue Nil
-
-          object SingleUseTraverser extends Traverser {
-            override def traverse(tree: Tree) {
-              tree match {
-                // assignment targets don't count as a dereference -- only check the rhs
-                case Assign(_, rhs) => traverse(rhs)
-                case tree: RefTree if tree.symbol != NoSymbol =>
-                  val sym = tree.symbol
-                  // println(s"$sym in ${sym.owner} from $currentOwner ($tree)")
-                  if ((sym.hasAccessorFlag || (sym.isTerm && !sym.isMethod)) && sym.isPrivate && !sym.isLazy && !sym.isModule // non-lazy private field or its accessor
-                    && !definitions.isPrimitiveValueClass(sym.tpe.resultType.typeSymbol) // primitives don't hang on to significant amounts of heap
-                    && sym.owner == currentOwner.enclClass && !(currentOwner.isGetter && currentOwner.accessed == sym)) {
-
-                    // println("added use in: " + currentOwner + " -- " + tree)
-                    usedIn(sym) ::= currentOwner
-                  }
-                  super.traverse(tree)
-                case _ => super.traverse(tree)
-              }
-            }
-          }
-          templStats foreach SingleUseTraverser.apply
-          // println("usedIn: " + usedIn)
-
-          // only consider usages from non-transient lazy vals (SI-9365)
-          val singlyUsedIn = usedIn filter { case (_, member :: Nil) => member.isLazy && !member.accessed.hasAnnotation(TransientAttr) case _ => false } toMap
-
-          // println("singlyUsedIn: " + singlyUsedIn)
-          singlyUsedIn
-        }
-
-        val map = mutable.Map[Symbol, Set[Symbol]]() withDefaultValue Set()
-        // invert the map to see which fields can be nulled for each non-transient lazy val
-        for ((field, users) <- singleUseFields; lazyFld <- users) map(lazyFld) += field
-
-        map.mapValues(_.toList sortBy (_.id)).toMap
-      }
-    }
-
-
     class SynthInitCheckedAccessorsIn(protected val clazz: Symbol) extends SynthCheckedAccessorsTreesInClass with CheckInitAccessorSymbolSynth {
       private object addInitBitsTransformer extends Transformer {
         private def checkedGetter(lhs: Tree)(pos: Position) = {
diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala
index aa2ccd9788..f66e00ce1a 100644
--- a/src/compiler/scala/tools/nsc/transform/Fields.scala
+++ b/src/compiler/scala/tools/nsc/transform/Fields.scala
@@ -659,7 +659,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
         val lazyVar = lazyVarOf(getter)
         val rhs = cast(Apply(selectSuper, Nil), lazyVar.info)
 
-        synthAccessorInClass.expandLazyClassMember(lazyVar, getter, rhs, Map.empty)
+        synthAccessorInClass.expandLazyClassMember(lazyVar, getter, rhs)
       }
 
       (afterOwnPhase { clazz.info.decls } toList) filter checkAndClearNeedsTrees map {
@@ -715,7 +715,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
             // note that `LazyAccessorTreeSynth` is pretty lightweight
             // (it's just a bunch of methods that all take a `clazz` parameter, which is thus stored as a field)
             val synthAccessorInClass = new SynthLazyAccessorsIn(currOwner)
-            synthAccessorInClass.expandLazyClassMember(lazyVarOf(statSym), statSym, transformedRhs, nullables.getOrElse(currOwner, Map.empty))
+            synthAccessorInClass.expandLazyClassMember(lazyVarOf(statSym), statSym, transformedRhs)
           }
 
         // drop the val for (a) constant (pure & not-stored) and (b) not-stored (but still effectful) fields
@@ -744,8 +744,6 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
       if (stat.isTerm) atOwner(exprOwner)(transform(stat))
       else transform(stat)
 
-    private val nullables = perRunCaches.newMap[Symbol, Map[Symbol, List[Symbol]]]
-
     override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = {
       val addedStats =
         if (!currentOwner.isClass || currentOwner.isPackageClass) Nil
@@ -756,10 +754,6 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
           else thickets
         }
 
-      val inRealClass = currentOwner.isClass && !(currentOwner.isPackageClass || currentOwner.isTrait)
-      if (inRealClass)
-        nullables(currentOwner) = lazyValNullables(currentOwner, stats)
-
       val newStats =
         stats mapConserve (if (exprOwner != currentOwner) transformTermsAtExprOwner(exprOwner) else transform)
 
diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index 582c51b90d..e62a12ce67 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -10,6 +10,7 @@ package transform
 import symtab._
 import Flags._
 import scala.annotation.tailrec
+import scala.collection.mutable
 
 
 abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthesis {
@@ -363,11 +364,13 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthes
     private val rootContext =
       erasure.NoContext.make(EmptyTree, rootMirror.RootClass, newScope)
 
+    private val nullables = mutable.AnyRefMap[Symbol, Map[Symbol, List[Symbol]]]()
 
     /** The first transform; called in a pre-order traversal at phase mixin
      *  (that is, every node is processed before its children).
      *  What transform does:
      *   - For every non-trait class, add all mixed in members to the class info.
+     *  - For every non-trait class, assign null to singly used private fields after use in lazy initialization.
      */
     private def preTransform(tree: Tree): Tree = {
       val sym = tree.symbol
@@ -381,12 +384,86 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthes
           else if (currentOwner.isTrait)
             publicizeTraitMethods(currentOwner)
 
+          if (!currentOwner.isTrait)
+            nullables(currentOwner) = lazyValNullables(currentOwner, body)
+
           tree
+        case dd: DefDef if dd.symbol.name.endsWith(nme.LAZY_SLOW_SUFFIX) =>
+          val fieldsToNull = nullables.getOrElse(sym.enclClass, Map()).getOrElse(sym, Nil)
+          if (fieldsToNull.isEmpty) dd
+          else {
+            deriveDefDef(dd) {
+              case blk@Block(stats, expr) =>
+                assert(dd.symbol.originalOwner.isClass, dd.symbol)
+                def nullify(sym: Symbol) =
+                  Select(gen.mkAttributedThis(sym.enclClass), sym.accessedOrSelf) === NULL
+                val stats1 = stats ::: fieldsToNull.map(nullify)
+                treeCopy.Block(blk, stats1, expr)
+              case tree =>
+                devWarning("Unexpected tree shape in lazy slow path")
+                tree
+            }
+          }
 
         case _ => tree
       }
     }
 
+    /** Map lazy values to the fields they should null after initialization. */
+    def lazyValNullables(clazz: Symbol, templStats: List[Tree]): Map[Symbol, List[Symbol]] = {
+      // if there are no lazy fields, take the fast path and save a traversal of the whole AST
+      if (!clazz.info.decls.exists(_.isLazy)) Map()
+      else {
+        // A map of single-use fields to the lazy value that uses them during initialization.
+        // Each field has to be private and defined in the enclosing class, and there must
+        // be exactly one lazy value using it.
+        //
+        // Such fields will be nulled after the initializer has memoized the lazy value.
+        val singleUseFields: Map[Symbol, List[Symbol]] = {
+          val usedIn = mutable.HashMap[Symbol, List[Symbol]]() withDefaultValue Nil
+
+          object SingleUseTraverser extends Traverser {
+            override def traverse(tree: Tree) {
+              tree match {
+                // assignment targets don't count as a dereference -- only check the rhs
+                case Assign(_, rhs) => traverse(rhs)
+                case tree: RefTree if tree.symbol != NoSymbol =>
+                  val sym = tree.symbol
+                  // println(s"$sym in ${sym.owner} from $currentOwner ($tree)")
+                  if ((sym.hasAccessorFlag || (sym.isTerm && !sym.isMethod)) && sym.isPrivate && !sym.isLazy && !sym.isModule // non-lazy private field or its accessor
+                    && !definitions.isPrimitiveValueClass(sym.tpe.resultType.typeSymbol) // primitives don't hang on to significant amounts of heap
+                    && sym.owner == currentOwner.enclClass && !(currentOwner.isGetter && currentOwner.accessed == sym)) {
+
+                    // println("added use in: " + currentOwner + " -- " + tree)
+                    usedIn(sym) ::= currentOwner
+                  }
+                  super.traverse(tree)
+                case _ => super.traverse(tree)
+              }
+            }
+          }
+          templStats foreach SingleUseTraverser.apply
+          // println("usedIn: " + usedIn)
+
+          // only consider usages from non-transient lazy vals (SI-9365)
+          val singlyUsedIn = usedIn.filter {
+            case (_, member :: Nil) if member.name.endsWith(nme.LAZY_SLOW_SUFFIX) =>
+              val lazyAccessor = member.owner.info.decl(member.name.stripSuffix(nme.LAZY_SLOW_SUFFIX))
+              !lazyAccessor.accessedOrSelf.hasAnnotation(TransientAttr)
+            case _ => false
+          }.toMap
+
+          // println("singlyUsedIn: " + singlyUsedIn)
+          singlyUsedIn
+        }
+
+        val map = mutable.Map[Symbol, Set[Symbol]]() withDefaultValue Set()
+        // invert the map to see which fields can be nulled for each non-transient lazy val
+        for ((field, users) <- singleUseFields; lazyFld <- users) map(lazyFld) += field
+
+        map.mapValues(_.toList sortBy (_.id)).toMap
+      }
+    }
 
     /** Add all new definitions to a non-trait class
       *
diff --git a/src/reflect/scala/reflect/internal/TypeDebugging.scala b/src/reflect/scala/reflect/internal/TypeDebugging.scala
index 63f897cd32..e9050b4e33 100644
--- a/src/reflect/scala/reflect/internal/TypeDebugging.scala
+++ b/src/reflect/scala/reflect/internal/TypeDebugging.scala
@@ -110,7 +110,7 @@ trait TypeDebugging {
         val hi_s = if (noPrint(hi)) "" else " <: " + ptTree(hi)
         lo_s + hi_s
       case _ if (t.symbol eq null) || (t.symbol eq NoSymbol) => to_s(t)
-      case _                                                 => "" + t.symbol.tpe
+      case _                                                 => "" + t.symbol.rawInfo.safeToString
     }
     def ptTypeParam(td: TypeDef): String = {
       val TypeDef(_, name, tparams, rhs) = td