[refactor] lazy val expansion in mixins/lazyvals

Towards expanding lazy vals and modules during fields phase.

3 files changed, 68 insertions, 103 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
index 5dddf30c96..ac47b3c464 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
@@ -234,16 +234,15 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL {
   }
 
   /** Return the synchronized part of the double-checked locking idiom around the syncBody tree. It guards with `cond` and
-   *  synchronizes on `clazz.this`. Additional statements can be included after initialization,
-   *  (outside the synchronized block).
-   *
-   *  The idiom works only if the condition is using a volatile field.
+    * synchronizes on `attrThis`. Additional statements can be included after initialization,
+    * (outside the synchronized block).
     *
-    *  @see http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
-   */
-  def mkSynchronizedCheck(clazz: Symbol, cond: Tree, syncBody: List[Tree], stats: List[Tree]): Tree =
-    mkSynchronizedCheck(mkAttributedThis(clazz), cond, syncBody, stats)
-
+    * The idiom works only if the condition is using a volatile field.
+    *
+    * @see http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
+    *
+    * TODO: update to state of the art on java 8 (https://shipilev.net/blog/2014/safe-public-construction/)
+    */
   def mkSynchronizedCheck(attrThis: Tree, cond: Tree, syncBody: List[Tree], stats: List[Tree]): Tree = {
     def blockOrStat(stats: List[Tree]): Tree = stats match {
       case head :: Nil => head
diff --git a/src/compiler/scala/tools/nsc/transform/LazyVals.scala b/src/compiler/scala/tools/nsc/transform/LazyVals.scala
index fc7999bf3b..ba2f6082e0 100644
--- a/src/compiler/scala/tools/nsc/transform/LazyVals.scala
+++ b/src/compiler/scala/tools/nsc/transform/LazyVals.scala
@@ -111,14 +111,14 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD
             debuglog(s"determined enclosing class/dummy/method for lazy val as $enclosingClassOrDummyOrMethod given symbol $sym")
             val idx = lazyVals(enclosingClassOrDummyOrMethod)
             lazyVals(enclosingClassOrDummyOrMethod) = idx + 1
-            val (rhs1, sDef) = mkLazyDef(enclosingClassOrDummyOrMethod, transform(rhs), idx, sym)
+            val (rhs1, sDef) = mkLazyLocalDef(enclosingClassOrDummyOrMethod, transform(rhs), idx, sym)
             sym.resetFlag((if (lazyUnit(sym)) 0 else LAZY) | ACCESSOR)
             (rhs1, sDef)
           } else if (sym.hasAllFlags(MODULE | METHOD) && !sym.owner.isTrait) {
             rhs match {
               case b @ Block((assign @ Assign(moduleRef, _)) :: Nil, expr) =>
                 def cond = Apply(Select(moduleRef, Object_eq), List(Literal(Constant(null))))
-                val (fastPath, slowPath) = mkFastPathBody(sym.owner.enclClass, moduleRef.symbol, cond, transform(assign) :: Nil, Nil, transform(expr))
+                val (fastPath, slowPath) = mkDoubleCheckedLocking(sym.owner.enclClass, moduleRef.symbol, cond, transform(assign) :: Nil, Nil, transform(expr))
                 (localTyper.typedPos(tree.pos)(fastPath), localTyper.typedPos(tree.pos)(slowPath))
               case rhs =>
                 global.reporter.error(tree.pos, "Unexpected tree on the RHS of a module accessor: " + rhs)
@@ -213,28 +213,27 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD
       }
     }
 
-    def mkSlowPathDef(clazz: Symbol, lzyVal: Symbol, cond: Tree, syncBody: List[Tree],
-                      stats: List[Tree], retVal: Tree): Tree = {
+    def mkDoubleCheckedLocking(clazz: Symbol, lzyVal: Symbol, cond: => Tree, syncBody: List[Tree], stats: List[Tree], retVal: Tree): (Tree, Tree) = {
       val owner = lzyVal.owner
       val defSym = owner.newMethod(nme.newLazyValSlowComputeName(lzyVal.name.toTermName), lzyVal.pos, STABLE | PRIVATE)
       defSym setInfo MethodType(List(), lzyVal.tpe.resultType)
       if (owner.isClass) owner.info.decls.enter(defSym)
-      debuglog(s"crete slow compute path $defSym with owner ${defSym.owner} for lazy val $lzyVal")
-      // this is a hack i don't understand for lazy vals nested in a lazy val, introduced in 3769f4d,
-      // tested in pos/t3670 (add9be64). class A { val n = { lazy val b = { lazy val dd = 3; dd }; b } }
-      // bitmaps has an entry bMethodSym -> List(bitmap$0), where bitmap$0.owner == bMethodSym.
-      // now we set bitmap$0.owner = b$lzycomputeMethodSym.
-      for (bitmap <- bitmaps(lzyVal)) bitmap.owner = defSym
-      val rhs: Tree = gen.mkSynchronizedCheck(clazz, cond, syncBody, stats).changeOwner(currentOwner -> defSym)
-
-      DefDef(defSym, addBitmapDefs(lzyVal, BLOCK(rhs, retVal)))
-    }
+
+      val slowPathDef: Tree = {
+        debuglog(s"crete slow compute path $defSym with owner ${defSym.owner} for lazy val $lzyVal")
+        // this is a hack i don't understand for lazy vals nested in a lazy val, introduced in 3769f4d,
+        // tested in pos/t3670 (add9be64). class A { val n = { lazy val b = { lazy val dd = 3; dd }; b } }
+        // bitmaps has an entry bMethodSym -> List(bitmap$0), where bitmap$0.owner == bMethodSym.
+        // now we set bitmap$0.owner = b$lzycomputeMethodSym.
+        for (bitmap <- bitmaps(lzyVal)) bitmap.owner = defSym
+
+        val rhs: Tree = gen.mkSynchronizedCheck(gen.mkAttributedThis(clazz), cond, syncBody, stats).changeOwner(currentOwner -> defSym)
+
+        DefDef(defSym, addBitmapDefs(lzyVal, BLOCK(rhs, retVal)))
+      }
 
 
-    def mkFastPathBody(clazz: Symbol, lzyVal: Symbol, cond: => Tree, syncBody: List[Tree],
-                       stats: List[Tree], retVal: Tree): (Tree, Tree) = {
-      val slowPathDef: Tree = mkSlowPathDef(clazz, lzyVal, cond, syncBody, stats, retVal)
-      (If(cond, Apply(Ident(slowPathDef.symbol), Nil), retVal), slowPathDef)
+      (If(cond, Apply(Ident(defSym), Nil), retVal), slowPathDef)
     }
 
     /** return a 'lazified' version of rhs. Rhs should conform to the
@@ -271,48 +270,41 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD
      *    }
      *  }
      */
-    private def mkLazyDef(methOrClass: Symbol, tree: Tree, offset: Int, lazyVal: Symbol): (Tree, Tree) = {
-      val bitmapSym           = getBitmapFor(methOrClass, offset)
-      val mask                = LIT(1 << (offset % FLAGS_PER_BYTE))
-      val bitmapRef = if (methOrClass.isClass) Select(This(methOrClass), bitmapSym) else Ident(bitmapSym)
+    val bitmaps = mutable.Map[Symbol, List[Symbol]]() withDefaultValue Nil
+    private def mkLazyLocalDef(methOrClass: Symbol, tree: Tree, offset: Int, lazyVal: Symbol): (Tree, Tree) = {
+      /** Return the symbol corresponding of the right bitmap int inside meth,
+       *  given offset.
+       */
+      val bitmapSym = {
+        val n = offset / FLAGS_PER_BYTE
+        val bmps = bitmaps(methOrClass)
+        if (bmps.length > n)
+          bmps(n)
+        else {
+          val sym = methOrClass.newVariable(nme.newBitmapName(nme.BITMAP_NORMAL, n), methOrClass.pos).setInfo(ByteTpe)
+          enteringTyper {
+            sym addAnnotation VolatileAttr
+          }
+
+          bitmaps(methOrClass) = (sym :: bmps).reverse
+          sym
+        }
+      }
 
-      def mkBlock(stmt: Tree) = BLOCK(stmt, mkSetFlag(bitmapSym, mask, bitmapRef), UNIT)
+      val mask = LIT(1 << (offset % FLAGS_PER_BYTE))
+      val bitmapRef = if (methOrClass.isClass) Select(This(methOrClass), bitmapSym) else Ident(bitmapSym)
 
       debuglog(s"create complete lazy def in $methOrClass for $lazyVal")
-      val (block, res) = tree match {
-        case Block(List(assignment), res) if !lazyUnit(lazyVal) =>
-          (mkBlock(assignment),  res)
-        case rhs                          =>
-          (mkBlock(rhs),         UNIT)
+
+      val (stmt, res) = tree match {
+        case Block(List(assignment), res) if !lazyUnit(lazyVal) => (assignment,  res)
+        case rhs                          => (rhs, UNIT)
       }
+      val block = Block(List(stmt, bitmapRef === (bitmapRef GEN_| (mask, bitmapKind))), UNIT)
 
       def cond = (bitmapRef GEN_& (mask, bitmapKind)) GEN_== (ZERO, bitmapKind)
-      val lazyDefs = mkFastPathBody(methOrClass.enclClass, lazyVal, cond, List(block), Nil, res)
+      val lazyDefs = mkDoubleCheckedLocking(methOrClass.enclClass, lazyVal, cond, List(block), Nil, res)
       (atPos(tree.pos)(localTyper.typed {lazyDefs._1 }), atPos(tree.pos)(localTyper.typed {lazyDefs._2 }))
     }
-
-    private def mkSetFlag(bmp: Symbol, mask: Tree, bmpRef: Tree): Tree =
-      bmpRef === (bmpRef GEN_| (mask, bitmapKind))
-
-    val bitmaps = mutable.Map[Symbol, List[Symbol]]() withDefaultValue Nil
-
-    /** Return the symbol corresponding of the right bitmap int inside meth,
-     *  given offset.
-     */
-    private def getBitmapFor(meth: Symbol, offset: Int): Symbol = {
-      val n = offset / FLAGS_PER_BYTE
-      val bmps = bitmaps(meth)
-      if (bmps.length > n)
-        bmps(n)
-      else {
-        val sym = meth.newVariable(nme.newBitmapName(nme.BITMAP_NORMAL, n), meth.pos).setInfo(ByteTpe)
-        enteringTyper {
-          sym addAnnotation VolatileAttr
-        }
-
-        bitmaps(meth) = (sym :: bmps).reverse
-        sym
-      }
-    }
   }
 }
diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index 4be611b747..f889f1244a 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -636,41 +636,6 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
         }
       }
 
-      def mkSlowPathDef(clazz: Symbol, lzyVal: Symbol, cond: Tree, syncBody: List[Tree],
-                        stats: List[Tree], retVal: Tree, attrThis: Tree, args: List[Tree]): Symbol = {
-        val defSym = clazz.newMethod(nme.newLazyValSlowComputeName(lzyVal.name.toTermName), lzyVal.pos, PRIVATE)
-        val params = defSym newSyntheticValueParams args.map(_.symbol.tpe)
-        defSym setInfoAndEnter MethodType(params, lzyVal.tpe.resultType)
-        val rhs: Tree = gen.mkSynchronizedCheck(attrThis, cond, syncBody, stats).changeOwner(currentOwner -> defSym)
-        val strictSubst = new TreeSymSubstituterWithCopying(args.map(_.symbol), params)
-        addDef(position(defSym), DefDef(defSym, strictSubst(BLOCK(rhs, retVal))))
-        defSym
-      }
-
-      def mkFastPathLazyBody(clazz: Symbol, lzyVal: Symbol, cond: => Tree, syncBody: List[Tree],
-                             stats: List[Tree], retVal: Tree): Tree = {
-        mkFastPathBody(clazz, lzyVal, cond, syncBody, stats, retVal, gen.mkAttributedThis(clazz), List())
-      }
-
-      def mkFastPathBody(clazz: Symbol, lzyVal: Symbol, cond: => Tree, syncBody: List[Tree],
-                        stats: List[Tree], retVal: Tree, attrThis: Tree, args: List[Tree]): Tree = {
-        val slowPathSym: Symbol = mkSlowPathDef(clazz, lzyVal, cond, syncBody, stats, retVal, attrThis, args)
-        If(cond, fn (This(clazz), slowPathSym, args.map(arg => Ident(arg.symbol)): _*), retVal)
-      }
-
-
-      /* Always copy the tree if we are going to perform sym substitution,
-       * otherwise we will side-effect on the tree that is used in the fast path
-       */
-      class TreeSymSubstituterWithCopying(from: List[Symbol], to: List[Symbol]) extends TreeSymSubstituter(from, to) {
-        override def transform(tree: Tree): Tree =
-          if (tree.hasSymbolField && from.contains(tree.symbol))
-            super.transform(tree.duplicate)
-          else super.transform(tree.duplicate)
-
-        override def apply[T <: Tree](tree: T): T = if (from.isEmpty) tree else super.apply(tree)
-      }
-
       /*  return a 'lazified' version of rhs. It uses double-checked locking to ensure
        *  initialization is performed at most once. For performance reasons the double-checked
        *  locking is split into two parts, the first (fast) path checks the bitmap without
@@ -708,20 +673,29 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
        *  If the class contains only a single lazy val then the bitmap is represented
        *  as a Boolean and the condition checking is a simple bool test.
        */
-      def mkLazyDef(clazz: Symbol, lzyVal: Symbol, init: List[Tree], retVal: Tree, offset: Int): Tree = {
+      def mkLazyMemberDef(clazz: Symbol, lzyVal: Symbol, init: List[Tree], retVal: Tree, offset: Int): Tree = {
         def nullify(sym: Symbol) = Select(This(clazz), sym.accessedOrSelf) === LIT(null)
 
         val bitmapSym = bitmapFor(clazz, offset, lzyVal)
         val kind      = bitmapKind(lzyVal)
         val mask      = maskForOffset(offset, lzyVal, kind)
-        def cond      = mkTest(clazz, mask, bitmapSym, equalToZero = true, kind)
         val nulls     = lazyValNullables(lzyVal).toList sortBy (_.id) map nullify
-        def syncBody  = init ::: List(mkSetFlag(clazz, offset, lzyVal, kind), UNIT)
 
         if (nulls.nonEmpty)
           log("nulling fields inside " + lzyVal + ": " + nulls)
 
-        typedPos(init.head.pos)(mkFastPathLazyBody(clazz, lzyVal, cond, syncBody, nulls, retVal))
+        val pos = if (lzyVal.pos != NoPosition) lzyVal.pos else clazz.pos // TODO: is the else branch ever taken?
+        val slowPathSym =
+          clazz.newMethod(nme.newLazyValSlowComputeName(lzyVal.name.toTermName), pos, PRIVATE) setInfoAndEnter MethodType(Nil, lzyVal.tpe.resultType)
+
+        def thisRef = gen.mkAttributedThis(clazz)
+        def cond    = mkTest(clazz, mask, bitmapSym, equalToZero = true, kind)
+
+        val statsToSynch  = init ::: List(mkSetFlag(clazz, offset, lzyVal, kind), UNIT)
+        val synchedRhs = gen.mkSynchronizedCheck(thisRef, cond, statsToSynch, nulls)
+        addDef(pos, DefDef(slowPathSym, Block(List(synchedRhs.changeOwner(currentOwner -> slowPathSym)), retVal)))
+
+        typedPos(init.head.pos)(If(cond, Apply(Select(thisRef, slowPathSym), Nil), retVal))
       }
 
       def mkCheckedAccessor(clazz: Symbol, retVal: Tree, offset: Int, pos: Position, fieldSym: Symbol): Tree = {
@@ -752,10 +726,10 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
           if (!clazz.isTrait && sym.isLazy && !isEmpty) {
             assert(fieldOffset contains sym, sym)
             deriveDefDef(stat) {
-              case t if isUnitGetter(sym) => mkLazyDef(clazz, sym, List(t), UNIT, fieldOffset(sym))
+              case t if isUnitGetter(sym) => mkLazyMemberDef(clazz, sym, List(t), UNIT, fieldOffset(sym))
 
               case Block(stats, res) =>
-                mkLazyDef(clazz, sym, stats, Select(This(clazz), res.symbol), fieldOffset(sym))
+                mkLazyMemberDef(clazz, sym, stats, Select(This(clazz), res.symbol), fieldOffset(sym))
 
               case t => t // pass specialized lazy vals through
             }