Opt: Get rid of the LiftTry phase; instead handle things in the back-…

…end.

When we enter a `try-catch` at the JVM level, we have to make sure
that the stack is empty. That's because, upon exception, the JVM
wipes the stack, and we must not lose operands that are already
on the stack that we will still use.

Previously, this was achieved with a transformation phase,
`LiftTry`, which lifted problematic `try-catch`es in local `def`s,
called `liftedTree$x`. It analyzed the tree to predict which
`try-catch`es would execute on a non-empty stack when eventually
compiled to the JVM.

This approach has several shortcomings.

It exhibits performance cliffs, as the generated def can then
cause more variables to be boxed in to `XRef`s. These were the
only extra defs created for implementation reasons rather than
for language reasons. As a user of the language, it is hard to
predict when such a lifted def will be needed.

The additional `liftedTree` methods also show up on stack traces
and obfuscate them. Debugging can be severely hampered as well.

Phases executing after `LiftTry`, notably `CapturedVars`, also had
to take care not to create more problematic situations as a result
of their transformations, which is hard to predict and to remember.

Finally, Scala.js and Scala Native do not have the same restriction,
so they received suboptimal code for no reason.

In this commit, we entirely remove the `LiftTry` phase. Instead, we
enhance the JVM back-end to deal with the situation. When starting
a `try-catch` on a non-empty stack, we stash the entire contents of
the stack into local variables. After the `try-catch`, we pop all
those local variables back onto the stack. We also null out the
leftover vars not to prevent garbage collection.

This new approach solves all of the problems mentioned above.

compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala

@@ -45,6 +45,8 @@ trait BCodeSkelBuilder extends BCodeHelpers {
     private var stack = new Array[BType](32)
     private var size = 0
 
+    def isEmpty: Boolean = size == 0
+
     def push(btype: BType): Unit =
       if size == stack.length then
         stack = java.util.Arrays.copyOf(stack, stack.length * 2)
@@ -81,6 +83,16 @@ trait BCodeSkelBuilder extends BCodeHelpers {
 
     def clear(): Unit =
       size = 0
+
+    def acquireFullStack(): IArray[BType] =
+      val res = IArray.unsafeFromArray(stack.slice(0, size))
+      size = 0
+      res
+
+    def restoreFullStack(fullStack: IArray[BType]): Unit =
+      assert(size == 0 && stack.length >= fullStack.length)
+      fullStack.copyToArray(stack)
+      size = fullStack.length
   end BTypesStack
 
   object BTypesStack:

compiler/src/dotty/tools/backend/jvm/BCodeSyncAndTry.scala

@@ -118,6 +118,11 @@ trait BCodeSyncAndTry extends BCodeBodyBuilder {
 
     /*
      *  Emitting try-catch is easy, emitting try-catch-finally not quite so.
+     *
+     *  For a try-catch, the only thing we need to care about is to stash the stack away
+     *  in local variables and load them back in afterwards, in case the incoming stack
+     *  is not empty.
+     *
      *  A finally-block (which always has type Unit, thus leaving the operand stack unchanged)
      *  affects control-transfer from protected regions, as follows:
      *
@@ -190,7 +195,7 @@ trait BCodeSyncAndTry extends BCodeBodyBuilder {
           }
         }
 
-      // ------ (0) locals used later ------
+      // ------ locals used later ------
 
       /*
        * `postHandlers` is a program point denoting:
@@ -203,6 +208,13 @@ trait BCodeSyncAndTry extends BCodeBodyBuilder {
        */
       val postHandlers = new asm.Label
 
+      // stack stash
+      val needStackStash = !stack.isEmpty && !caseHandlers.isEmpty
+      val acquiredStack = if needStackStash then stack.acquireFullStack() else null
+      val stashLocals =
+        if acquiredStack == null then null
+        else acquiredStack.uncheckedNN.filter(_ != UNIT).map(btpe => locals.makeTempLocal(btpe))
+
       val hasFinally   = (finalizer != tpd.EmptyTree)
 
       /*
@@ -222,6 +234,17 @@ trait BCodeSyncAndTry extends BCodeBodyBuilder {
        */
       val finCleanup   = if (hasFinally) new asm.Label else null
 
+      /* ------ (0) Stash the stack into local variables, if necessary.
+       *            From top of the stack down to the bottom.
+       * ------
+       */
+
+      if stashLocals != null then
+        val stashLocalsNN = stashLocals.uncheckedNN // why is this necessary?
+        for i <- (stashLocalsNN.length - 1) to 0 by -1 do
+          val local = stashLocalsNN(i)
+          bc.store(local.idx, local.tk)
+
       /* ------ (1) try-block, protected by:
        *                       (1.a) the EHs due to case-clauses,   emitted in (2),
        *                       (1.b) the EH  due to finally-clause, emitted in (3.A)
@@ -367,6 +390,37 @@ trait BCodeSyncAndTry extends BCodeBodyBuilder {
         emitFinalizer(finalizer, tmp, isDuplicate = false) // the only invocation of emitFinalizer with `isDuplicate == false`
       }
 
+      /* ------ (5) Unstash the stack, if it was stashed before.
+       *            From bottom of the stack to the top.
+       *            If there is a non-UNIT result, we need to temporarily store
+       *            that one in a local variable while we unstash.
+       * ------
+       */
+
+      if stashLocals != null then
+        val stashLocalsNN = stashLocals.uncheckedNN // why is this necessary?
+
+        val resultLoc =
+          if kind == UNIT then null
+          else if tmp != null then locals(tmp) // reuse the same local
+          else locals.makeTempLocal(kind)
+        if resultLoc != null then
+          bc.store(resultLoc.idx, kind)
+
+        for i <- 0 until stashLocalsNN.size do
+          val local = stashLocalsNN(i)
+          bc.load(local.idx, local.tk)
+          if local.tk.isRef then
+            bc.emit(asm.Opcodes.ACONST_NULL)
+            bc.store(local.idx, local.tk)
+
+        stack.restoreFullStack(acquiredStack.nn)
+        bc.load(resultLoc.idx, kind)
+        if kind.isRef then
+          bc.emit(asm.Opcodes.ACONST_NULL)
+          bc.store(resultLoc.idx, kind)
+      end if // stashLocals != null
+
       kind
     } // end of genLoadTry()
 

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -100,7 +100,6 @@ class Compiler {
          new Getters,                // Replace non-private vals and vars with getter defs (fields are added later)
          new SpecializeFunctions,    // Specialized Function{0,1,2} by replacing super with specialized super
          new SpecializeTuples,       // Specializes Tuples by replacing tuple construction and selection trees
-         new LiftTry,                // Put try expressions that might execute on non-empty stacks into their own methods
          new CollectNullableFields,  // Collect fields that can be nulled out after use in lazy initialization
          new ElimOuterSelect,        // Expand outer selections
          new ResolveSuper,           // Implement super accessors

compiler/src/dotty/tools/dotc/core/NameKinds.scala

@@ -316,7 +316,6 @@ object NameKinds {
   val TailTempName: UniqueNameKind           = new UniqueNameKind("$tmp")
   val ExceptionBinderName: UniqueNameKind    = new UniqueNameKind("ex")
   val SkolemName: UniqueNameKind             = new UniqueNameKind("?")
-  val LiftedTreeName: UniqueNameKind         = new UniqueNameKind("liftedTree")
   val SuperArgName: UniqueNameKind           = new UniqueNameKind("$superArg$")
   val DocArtifactName: UniqueNameKind        = new UniqueNameKind("$doc")
   val UniqueInlineName: UniqueNameKind       = new UniqueNameKind("$i")

compiler/src/dotty/tools/dotc/transform/CapturedVars.scala

@@ -25,9 +25,6 @@ class CapturedVars extends MiniPhase with IdentityDenotTransformer:
 
   override def description: String = CapturedVars.description
 
-  override def runsAfterGroupsOf: Set[String] = Set(LiftTry.name)
-    // lifting tries changes what variables are considered to be captured
-
   private[this] var Captured: Store.Location[util.ReadOnlySet[Symbol]] = _
   private def captured(using Context) = ctx.store(Captured)
 
@@ -131,42 +128,15 @@ class CapturedVars extends MiniPhase with IdentityDenotTransformer:
    *
    *      intRef.elem = expr
    *
-   *  rewrite using a temporary var to
-   *
-   *      val ev$n = expr
-   *      intRef.elem = ev$n
-   *
-   *  That way, we avoid the problem that `expr` might contain a `try` that would
-   *  run on a non-empty stack (which is illegal under JVM rules). Note that LiftTry
-   *  has already run before, so such `try`s would not be eliminated.
-   *
-   *  If the ref type lhs is followed by a cast (can be an artifact of nested translation),
-   *  drop the cast.
-   *
-   *  If the ref type is `ObjectRef` or `VolatileObjectRef`, immediately assign `null`
-   *  to the temporary to make the underlying target of the reference available for
-   *  garbage collection. Nullification is omitted if the `expr` is already `null`.
-   *
-   *      var ev$n: RHS = expr
-   *      objRef.elem = ev$n
-   *      ev$n = null.asInstanceOf[RHS]
+   *  the lhs can be followed by a cast as an artifact of nested translation.
+   *  In that case, drop the cast.
    */
   override def transformAssign(tree: Assign)(using Context): Tree =
-    def absolved: Boolean = tree.rhs match
-      case Literal(Constant(null)) | Typed(Literal(Constant(null)), _) => true
-      case _ => false
-    def recur(lhs: Tree): Tree = lhs match
+    tree.lhs match
       case TypeApply(Select(qual@Select(_, nme.elem), nme.asInstanceOf_), _) =>
-        recur(qual)
-      case Select(_, nme.elem) if refInfo.boxedRefClasses.contains(lhs.symbol.maybeOwner) =>
-        val tempDef = transformFollowing(SyntheticValDef(TempResultName.fresh(), tree.rhs, flags = Mutable))
-        val update  = cpy.Assign(tree)(lhs, ref(tempDef.symbol))
-        def reset   = cpy.Assign(tree)(ref(tempDef.symbol), nullLiteral.cast(tempDef.symbol.info))
-        val res     = if refInfo.objectRefClasses(lhs.symbol.maybeOwner) && !absolved then reset else unitLiteral
-        transformFollowing(Block(tempDef :: update :: Nil, res))
+        cpy.Assign(tree)(qual, tree.rhs)
       case _ =>
         tree
-    recur(tree.lhs)
 
 object CapturedVars:
   val name: String = "capturedVars"

docs/_docs/internals/overall-structure.md

@@ -151,7 +151,6 @@ phases. The current list of phases is specified in class [Compiler] as follows:
          new InterceptedMethods,     // Special handling of `==`, `|=`, `getClass` methods
          new Getters,                // Replace non-private vals and vars with getter defs (fields are added later)
          new SpecializeFunctions,    // Specialized Function{0,1,2} by replacing super with specialized super
-         new LiftTry,                // Put try expressions that might execute on non-empty stacks into their own methods
          new CollectNullableFields,  // Collect fields that can be nulled out after use in lazy initialization
          new ElimOuterSelect,        // Expand outer selections
          new ResolveSuper,           // Implement super accessors

tests/neg/t1672b.scala

@@ -41,7 +41,6 @@ object Test1772B {
     }
   }
 
-  // the `liftedTree` local method will prevent a tail call here.
   @tailrec
   def bar(i : Int) : Int = {
     if (i == 0) 0

tests/run/i1692.scala

@@ -22,7 +22,7 @@ class LazyNullable(a: => Int) {
   lazy val l4Inf = eInf
 
   private[this] val i = "I"
-  // null out i even though the try ends up lifted, because the LazyVals phase runs before the LiftTry phase
+  // null out i even though the try needs stack stashing
   lazy val l5 = try i catch { case e: Exception => () }
 }
 

tests/run/i1692b.scala

@@ -24,7 +24,7 @@ class LazyNullable(a: => Int) {
   @threadUnsafe lazy val l4Inf = try eInf finally () // null out e, since private[this] is inferred
 
   private[this] val i = "I"
-  // null out i even though the try ends up lifted, because the LazyVals phase runs before the LiftTry phase
+  // null out i even though the try needs stack stashing
   @threadUnsafe lazy val l5 = try i catch { case e: Exception => () }
 }