SI-6863 root cause fixed using factory of scala.runtime.*Ref

This commit does away with an error-prone division of labor between
UnCurry and LambdaLift, a division of labor by which UnCurry had to
anticipate under which circumstances LambdaLift creates a
scala.runtime.*Ref whose initial value is given by a an expression
including a Try in non-statement position. That sounds complicated,
and it is.

The solution so far (fixing SI-6863) is replaced by a simpler approach,
at the cost of forward binary comptability with pre-2.11 releases,
this time fixing the root cause of SI-6863.

From now on, a s.r.*Ref is instantiated via invocation of
a static factory method in the s.r.*Ref class in question.
Unlike the code that was emitted so far (which involved
NEW refclass, DUP, expr, INVOKESPECIAL refclass.<init>)
the "expr" doesn't appear on the operand stack on top
of the *Ref value being initialized. In other words,
the *Ref initialization is in statement position provided "expr" is.

1 files changed, 27 insertions, 47 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
index 6ff8792a45..8c4663cd48 100644
--- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
+++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
@@ -32,6 +32,21 @@ abstract class LambdaLift extends InfoTransform {
     }
   }
 
+  /** scala.runtime.*Ref classes */
+  private lazy val allRefClasses: Set[Symbol] = {
+    refClass.values.toSet ++ volatileRefClass.values.toSet ++ Set(VolatileObjectRefClass, ObjectRefClass)
+  }
+
+  /** Each scala.runtime.*Ref class has a static method `create(value)` that simply instantiates the Ref to carry that value. */
+  private lazy val refCreateMethod: Map[Symbol, Symbol] = {
+    mapFrom(allRefClasses.toList)(x => getMemberMethod(x.companionModule, nme.create))
+  }
+
+  /** Quite frequently a *Ref is initialized with its zero (e.g., null, 0.toByte, etc.) Method `zero()` of *Ref class encapsulates that pattern. */
+  private lazy val refZeroMethod: Map[Symbol, Symbol] = {
+    mapFrom(allRefClasses.toList)(x => getMemberMethod(x.companionModule, nme.zero))
+  }
+
   def transformInfo(sym: Symbol, tp: Type): Type =
     if (sym.isCapturedVariable) capturedVariableType(sym, tpe = lifted(tp), erasedTypes = true)
     else lifted(tp)
@@ -444,56 +459,21 @@ abstract class LambdaLift extends InfoTransform {
         case ValDef(mods, name, tpt, rhs) =>
           if (sym.isCapturedVariable) {
             val tpt1 = TypeTree(sym.tpe) setPos tpt.pos
-            /* Creating a constructor argument if one isn't present. */
-            val constructorArg = rhs match {
+
+            val refTypeSym = sym.tpe.typeSymbol
+
+            val factoryCall = typer.typedPos(rhs.pos) {
+              rhs match {
               case EmptyTree =>
-                sym.tpe.typeSymbol.primaryConstructor.info.paramTypes match {
-                  case List(tp) => gen.mkZero(tp)
-                  case _        =>
-                    debugwarn("Couldn't determine how to properly construct " + sym)
-                    rhs
-                }
-              case arg => arg
+                val zeroMSym   = refZeroMethod(refTypeSym)
+                gen.mkMethodCall(zeroMSym, Nil)
+              case arg =>
+                val createMSym = refCreateMethod(refTypeSym)
+                gen.mkMethodCall(createMSym, arg :: Nil)
+              }
             }
 
-            /* Wrap expr argument in new *Ref(..) constructor. But try/catch
-             * is a problem because a throw will clear the stack and post catch
-             * we would expect the partially-constructed object to be on the stack
-             * for the call to init. So we recursively
-             * search for "leaf" result expressions where we know its safe
-             * to put the new *Ref(..) constructor or, if all else fails, transform
-             * an expr to { val temp=expr; new *Ref(temp) }.
-             * The reason we narrowly look for try/catch in captured var definitions
-             * is because other try/catch expression have already been lifted
-             * see SI-6863
-             */
-            def refConstr(expr: Tree): Tree = typer.typedPos(expr.pos)(expr match {
-              // very simple expressions can be wrapped in a new *Ref(expr) because they can't have
-              // a try/catch in final expression position.
-              case Ident(_) | Apply(_, _) | Literal(_) | New(_) | Select(_, _) | Throw(_) | Assign(_, _) | ValDef(_, _, _, _) | Return(_) | EmptyTree =>
-                New(sym.tpe, expr)
-              case Try(block, catches, finalizer) =>
-                Try(refConstr(block), catches map refConstrCase, finalizer)
-              case Block(stats, expr) =>
-                Block(stats, refConstr(expr))
-              case If(cond, trueBranch, falseBranch) =>
-                If(cond, refConstr(trueBranch), refConstr(falseBranch))
-              case Match(selector, cases) =>
-                Match(selector, cases map refConstrCase)
-              // if we can't figure out what else to do, turn expr into {val temp1 = expr; new *Ref(temp1)} to avoid
-              // any possibility of try/catch in the *Ref constructor. This should be a safe tranformation as a default
-              // though it potentially wastes a variable slot. In particular this case handles LabelDefs.
-              case _ =>
-                debuglog("assigning expr to temp: " + (expr.pos))
-                val tempSym = currentOwner.newValue(unit.freshTermName("temp"), expr.pos) setInfo expr.tpe
-                val tempDef = ValDef(tempSym, expr) setPos expr.pos
-                val tempRef = Ident(tempSym) setPos expr.pos
-                Block(tempDef, New(sym.tpe, tempRef))
-            })
-            def refConstrCase(cdef: CaseDef): CaseDef =
-              CaseDef(cdef.pat, cdef.guard, refConstr(cdef.body))
-
-            treeCopy.ValDef(tree, mods, name, tpt1, refConstr(constructorArg))
+            treeCopy.ValDef(tree, mods, name, tpt1, factoryCall)
           } else tree
         case Return(Block(stats, value)) =>
           Block(stats, treeCopy.Return(tree, value)) setType tree.tpe setPos tree.pos