Fix scala#14773: Reuse the param slots for the tailrec local mutable vars.

The tailrec phase generates code that looks like

  class Bar {
    def foo(x1: Int, x2: Int): Int = {
      var this$: Bar = this;
      var taillocal$x1: Int = x1;
      var taillocal$x2: Int = x2;
      ...
      // body where `this`, `x1` and `x2` are replaced by
      // `this$`, `taillocal$x1` and `taillocal$x2`
    }
  }

This generates bytecode where the `this` value and the parameters
never actually change, and are never used. Instead, the synthetic
mutable variables are used instead.

As described in the linked issue, this confuses debuggers, which
only display the never-changing original `this` value and
parameters.

In this commit, we intercept this shape of code in the back-end.
We reliable identify tailrec-generated `ValDef`s from their
semantic names, with an additional safety check that they are
`Synthetic | Mutable`. When we find this shape, we do not allocate
local slots for the `var`s, and instead reuse the slots for `this`
and the parameters. We skip past the `ValDef`s so that code
generation does not re-emit useless assignments.

Author: sjrd

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

@@ -4,6 +4,8 @@ package jvm
 
 import scala.language.unsafeNulls
 
+import scala.annotation.tailrec
+
 import scala.collection.{ mutable, immutable }
 
 import scala.tools.asm
@@ -484,6 +486,14 @@ trait BCodeSkelBuilder extends BCodeHelpers {
         slots.getOrElse(locSym, makeLocal(locSym))
       }
 
+      def reuseLocal(sym: Symbol, loc: Local): Unit =
+        val existing = slots.put(sym, loc)
+        if (existing.isDefined)
+          report.error("attempt to create duplicate local var.", ctx.source.atSpan(sym.span))
+
+      def reuseThisSlot(sym: Symbol): Unit =
+        reuseLocal(sym, Local(symInfoTK(sym), sym.javaSimpleName, 0, sym.is(Synthetic)))
+
       private def makeLocal(sym: Symbol, tk: BType): Local = {
         assert(nxtIdx != -1, "not a valid start index")
         val loc = Local(tk, sym.javaSimpleName, nxtIdx, sym.is(Synthetic))
@@ -753,18 +763,47 @@ trait BCodeSkelBuilder extends BCodeHelpers {
           .addFlagIf(isNative, asm.Opcodes.ACC_NATIVE) // native methods of objects are generated in mirror classes
 
       // TODO needed? for(ann <- m.symbol.annotations) { ann.symbol.initialize }
-      initJMethod(flags, params.map(_.symbol))
+      val paramSyms = params.map(_.symbol)
+      initJMethod(flags, paramSyms)
 
 
       if (!isAbstractMethod && !isNative) {
+        // #14773 Reuse locals slots for tailrec-generated mutable vars
+        val trimmedRhs: Tree =
+          @tailrec def loop(stats: List[Tree]): List[Tree] =
+            stats match
+              case (tree @ ValDef(TailLocalName(_, _), _, _)) :: rest if tree.symbol.isAllOf(Mutable | Synthetic) =>
+                tree.rhs match
+                  case This(_) =>
+                    locals.reuseThisSlot(tree.symbol)
+                    loop(rest)
+                  case rhs: Ident if paramSyms.contains(rhs.symbol) =>
+                    locals.reuseLocal(tree.symbol, locals(rhs.symbol))
+                    loop(rest)
+                  case _ =>
+                    stats
+              case _ =>
+                stats
+          end loop
+
+          rhs match
+            case Block(stats, expr) =>
+              val trimmedStats = loop(stats)
+              if trimmedStats eq stats then
+                rhs
+              else
+                Block(trimmedStats, expr)
+            case _ =>
+              rhs
+        end trimmedRhs
 
         def emitNormalMethodBody(): Unit = {
           val veryFirstProgramPoint = currProgramPoint()
-          genLoad(rhs, returnType)
+          genLoad(trimmedRhs, returnType)
 
-          rhs match {
+          trimmedRhs match {
             case (_: Return) | Block(_, (_: Return)) => ()
-            case (_: Apply) | Block(_, (_: Apply)) if rhs.symbol eq defn.throwMethod => ()
+            case (_: Apply) | Block(_, (_: Apply)) if trimmedRhs.symbol eq defn.throwMethod => ()
             case tpd.EmptyTree =>
               report.error("Concrete method has no definition: " + dd + (
                 if (ctx.settings.Ydebug.value) "(found: " + methSymbol.owner.info.decls.toList.mkString(", ") + ")"

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

@@ -253,7 +253,7 @@ class TailRec extends MiniPhase {
           val tpe =
             if (enclosingClass.is(Module)) enclosingClass.thisType
             else enclosingClass.classInfo.selfType
-          val sym = newSymbol(method, nme.SELF, Synthetic | Mutable, tpe)
+          val sym = newSymbol(method, TailLocalName.fresh(nme.SELF), Synthetic | Mutable, tpe)
           varForRewrittenThis = Some(sym)
           sym
       }

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala

@@ -971,41 +971,35 @@ class TestBCode extends DottyBytecodeTest {
       val factMeth = getMethod(fooClass, "fact")
 
       assertSameCode(factMeth, List(
+        Label(0),
+        VarOp(ILOAD, 1),
+        Op(ICONST_0),
+        Jump(IF_ICMPNE, Label(7)),
+        VarOp(ILOAD, 2),
+        Jump(GOTO, Label(26)),
+        Label(7),
         VarOp(ALOAD, 0),
         VarOp(ASTORE, 3),
-        VarOp(ILOAD, 2),
+        VarOp(ILOAD, 1),
+        Op(ICONST_1),
+        Op(ISUB),
         VarOp(ISTORE, 4),
+        VarOp(ILOAD, 2),
         VarOp(ILOAD, 1),
+        Op(IMUL),
         VarOp(ISTORE, 5),
-        Label(6),
-        VarOp(ILOAD, 5),
-        Op(ICONST_0),
-        Jump(IF_ICMPNE, Label(13)),
-        VarOp(ILOAD, 4),
-        Jump(GOTO, Label(32)),
-        Label(13),
         VarOp(ALOAD, 3),
-        VarOp(ASTORE, 6),
-        VarOp(ILOAD, 5),
-        Op(ICONST_1),
-        Op(ISUB),
-        VarOp(ISTORE, 7),
+        VarOp(ASTORE, 0),
         VarOp(ILOAD, 4),
+        VarOp(ISTORE, 1),
         VarOp(ILOAD, 5),
-        Op(IMUL),
-        VarOp(ISTORE, 8),
-        VarOp(ALOAD, 6),
-        VarOp(ASTORE, 3),
-        VarOp(ILOAD, 7),
-        VarOp(ISTORE, 5),
-        VarOp(ILOAD, 8),
-        VarOp(ISTORE, 4),
-        Jump(GOTO, Label(35)),
-        Label(32),
+        VarOp(ISTORE, 2),
+        Jump(GOTO, Label(29)),
+        Label(26),
         Op(IRETURN),
-        Label(35),
-        Jump(GOTO, Label(6)),
-        Op(ATHROW),
+        Label(29),
+        Jump(GOTO, Label(0)),
+        Op(NOP),
         Op(ATHROW),
       ))
 
@@ -1015,39 +1009,35 @@ class TestBCode extends DottyBytecodeTest {
       val sumMeth = getMethod(intListClass, "sum")
 
       assertSameCode(sumMeth, List(
+        Label(0),
         VarOp(ALOAD, 0),
-        VarOp(ASTORE, 2),
-        VarOp(ILOAD, 1),
-        VarOp(ISTORE, 3),
-        Label(4),
-        VarOp(ALOAD, 2),
         Field(GETFIELD, "IntList", "tail", "LIntList;"),
-        VarOp(ASTORE, 4),
-        VarOp(ALOAD, 4),
-        Jump(IFNONNULL, Label(16)),
-        VarOp(ILOAD, 3),
+        VarOp(ASTORE, 2),
         VarOp(ALOAD, 2),
+        Jump(IFNONNULL, Label(12)),
+        VarOp(ILOAD, 1),
+        VarOp(ALOAD, 0),
         Field(GETFIELD, "IntList", "head", "I"),
         Op(IADD),
-        Jump(GOTO, Label(30)),
-        Label(16),
-        VarOp(ALOAD, 4),
-        VarOp(ASTORE, 5),
-        VarOp(ILOAD, 3),
+        Jump(GOTO, Label(26)),
+        Label(12),
         VarOp(ALOAD, 2),
+        VarOp(ASTORE, 3),
+        VarOp(ILOAD, 1),
+        VarOp(ALOAD, 0),
         Field(GETFIELD, "IntList", "head", "I"),
         Op(IADD),
-        VarOp(ISTORE, 6),
-        VarOp(ALOAD, 5),
-        VarOp(ASTORE, 2),
-        VarOp(ILOAD, 6),
-        VarOp(ISTORE, 3),
-        Jump(GOTO, Label(33)),
-        Label(30),
+        VarOp(ISTORE, 4),
+        VarOp(ALOAD, 3),
+        VarOp(ASTORE, 0),
+        VarOp(ILOAD, 4),
+        VarOp(ISTORE, 1),
+        Jump(GOTO, Label(29)),
+        Label(26),
         Op(IRETURN),
-        Label(33),
-        Jump(GOTO, Label(4)),
-        Op(ATHROW),
+        Label(29),
+        Jump(GOTO, Label(0)),
+        Op(NOP),
         Op(ATHROW),
       ))
     }