Improve source positions emited for synthetic unit in if-conditions (#20431)

Fixes #18238 

Source positions (lines) emitted for synthetic unit in if-conditions
were incorrect, because they were missing explicit line position for the
else condition (introduced by the compiler as synthetic unit) the
debugger when stepping into the `else` branch it was using the last
position of the `then` expression. Now, we explicitly use the line
position of the condition for the synthetic `else` branch - it matches
the behaviour of Scala 2.

* Use `SyntheticUnit` and introduce `untpd.syntheticUnitLiteral` to
detect if `else` branch is defined - allows to emit correct positions
for the explicit unit `else` branch

Author: sjrd

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

@@ -24,6 +24,7 @@ import dotty.tools.dotc.core.Contexts.*
 import dotty.tools.dotc.core.Phases.*
 import dotty.tools.dotc.core.Decorators.em
 import dotty.tools.dotc.report
+import dotty.tools.dotc.ast.Trees.SyntheticUnit
 
 /*
  *
@@ -218,10 +219,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
       val success = new asm.Label
       val failure = new asm.Label
 
-      val hasElse = !elsep.isEmpty && (elsep match {
-        case Literal(value) if value.tag == UnitTag => false
-        case _ => true
-      })
+      val hasElse = !elsep.hasAttachment(SyntheticUnit)
 
       genCond(condp, success, failure, targetIfNoJump = success)
       markProgramPoint(success)
@@ -250,6 +248,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
         if hasElse then
           genLoadTo(elsep, expectedType, dest)
         else
+          lineNumber(tree.cond)
           genAdaptAndSendToDest(UNIT, expectedType, dest)
         expectedType
       end if

compiler/src/dotty/tools/dotc/ast/Desugar.scala

@@ -188,7 +188,7 @@ object desugar {
     if isSetterNeeded(vdef) then
       val setterParam = makeSyntheticParameter(tpt = SetterParamTree().watching(vdef))
       // The rhs gets filled in later, when field is generated and getter has parameters (see Memoize miniphase)
-      val setterRhs = if (vdef.rhs.isEmpty) EmptyTree else unitLiteral
+      val setterRhs = if (vdef.rhs.isEmpty) EmptyTree else syntheticUnitLiteral
       val setter = cpy.DefDef(vdef)(
           name    = valName.setterName,
           paramss = (setterParam :: Nil) :: Nil,
@@ -1489,7 +1489,7 @@ object desugar {
   def block(tree: Block)(using Context): Block = tree.expr match {
     case EmptyTree =>
       cpy.Block(tree)(tree.stats,
-        unitLiteral.withSpan(if (tree.stats.isEmpty) tree.span else tree.span.endPos))
+        syntheticUnitLiteral.withSpan(if (tree.stats.isEmpty) tree.span else tree.span.endPos))
     case _ =>
       tree
   }
@@ -2017,7 +2017,7 @@ object desugar {
           case ts: Thicket => ts.trees.tail
           case t => Nil
         } map {
-          case Block(Nil, EmptyTree) => unitLiteral // for s"... ${} ..."
+          case Block(Nil, EmptyTree) => syntheticUnitLiteral // for s"... ${} ..."
           case Block(Nil, expr) => expr // important for interpolated string as patterns, see i1773.scala
           case t => t
         }
@@ -2050,7 +2050,7 @@ object desugar {
         val pats1 = if (tpt.isEmpty) pats else pats map (Typed(_, tpt))
         flatTree(pats1 map (makePatDef(tree, mods, _, rhs)))
       case ext: ExtMethods =>
-        Block(List(ext), unitLiteral.withSpan(ext.span))
+        Block(List(ext), syntheticUnitLiteral.withSpan(ext.span))
       case f: FunctionWithMods if f.hasErasedParams => makeFunctionWithValDefs(f, pt)
     }
     desugared.withSpan(tree.span)

compiler/src/dotty/tools/dotc/ast/untpd.scala

@@ -495,7 +495,11 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
   def InferredTypeTree(tpe: Type)(using Context): TypedSplice =
     TypedSplice(new InferredTypeTree().withTypeUnchecked(tpe))
 
-  def unitLiteral(implicit src: SourceFile): Literal = Literal(Constant(())).withAttachment(SyntheticUnit, ())
+  def unitLiteral(implicit src: SourceFile): Literal =
+    Literal(Constant(()))
+
+  def syntheticUnitLiteral(implicit src: SourceFile): Literal =
+    unitLiteral.withAttachment(SyntheticUnit, ())
 
   def ref(tp: NamedType)(using Context): Tree =
     TypedSplice(tpd.ref(tp))

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -1725,7 +1725,7 @@ object Parsers {
             case arg =>
               arg
           val args1 = args.mapConserve(sanitize)
-          
+
           if in.isArrow || isPureArrow || erasedArgs.contains(true) then
             functionRest(args)
           else
@@ -2424,7 +2424,7 @@ object Parsers {
               in.nextToken();
               val expr = subExpr()
               if expr.span.exists then expr
-              else unitLiteral // finally without an expression
+              else syntheticUnitLiteral // finally without an expression
             }
             else {
               if handler.isEmpty then
@@ -3921,10 +3921,10 @@ object Parsers {
             val stats = selfInvocation() :: (
               if (isStatSep) { in.nextToken(); blockStatSeq() }
               else Nil)
-            Block(stats, unitLiteral)
+            Block(stats, syntheticUnitLiteral)
           }
         }
-      else Block(selfInvocation() :: Nil, unitLiteral)
+      else Block(selfInvocation() :: Nil, syntheticUnitLiteral)
 
     /** SelfInvocation  ::= this ArgumentExprs {ArgumentExprs}
      */

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -2256,7 +2256,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
                             // because we do not know the internal type params and method params.
                             // Hence no adaptation is possible, and we assume WildcardType as prototype.
         (from, proto)
-    val expr1 = typedExpr(tree.expr orElse untpd.unitLiteral.withSpan(tree.span), proto)
+    val expr1 = typedExpr(tree.expr orElse untpd.syntheticUnitLiteral.withSpan(tree.span), proto)
     assignType(cpy.Return(tree)(expr1, from))
   end typedReturn
 

compiler/src/dotty/tools/repl/ReplCompiler.scala

@@ -159,7 +159,7 @@ class ReplCompiler extends Compiler:
       def wrap(trees: List[untpd.Tree]): untpd.PackageDef = {
         import untpd.*
 
-        val valdef = ValDef("expr".toTermName, TypeTree(), Block(trees, unitLiteral).withSpan(Span(0, expr.length)))
+        val valdef = ValDef("expr".toTermName, TypeTree(), Block(trees, syntheticUnitLiteral).withSpan(Span(0, expr.length)))
         val tmpl = Template(emptyConstructor, Nil, Nil, EmptyValDef, List(valdef))
         val wrapper = TypeDef("$wrapper".toTypeName, tmpl)
           .withMods(Modifiers(Final))

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

@@ -0,0 +1,116 @@
+package dotty.tools.backend.jvm
+
+import scala.language.unsafeNulls
+
+import org.junit.Assert._
+import org.junit.Test
+
+class SourcePositionsTest extends DottyBytecodeTest:
+  import ASMConverters._
+
+  @Test def issue18238_a(): Unit = {
+    val code =
+      """
+      |class Test {
+      |  def test(): Unit = {
+      |    var x = 3
+      |    var y = 2
+      |    while(true) {
+      |      if (x < y)
+      |        if (x >= y)
+      |          x += 1
+      |        else
+      |          y -= 1
+      |    }
+      |  }
+      |}""".stripMargin
+
+    checkBCode(code) { dir =>
+      val testClass = loadClassNode(dir.lookupName("Test.class", directory = false).input, skipDebugInfo = false)
+      val testMethod = getMethod(testClass, "test")
+      val lineNumbers = instructionsFromMethod(testMethod).collect{case ln: LineNumber => ln}
+      val expected = List(
+        LineNumber(4, Label(0)),   // var x
+        LineNumber(5, Label(4)),   // var y
+        LineNumber(6, Label(8)),   // while(true)
+        LineNumber(7, Label(13)),  // if (x < y)
+        LineNumber(8, Label(18)),  //   if (x >= y)
+        LineNumber(9, Label(23)),  //     x += 1
+        LineNumber(11, Label(27)), //     y -= 1
+        LineNumber(7, Label(32))   // <synthetic unit> point back to `if (x < y)
+      )
+      assertEquals(expected, lineNumbers)
+    }
+  }
+
+  @Test  def issue18238_b(): Unit = {
+    val code =
+      """
+      |class Test {
+      |  def test(): Unit = {
+      |    var x = 3
+      |    var y = 2
+      |    while(true) {
+      |      if (x < y)
+      |        if (x >= y)
+      |          x += 1
+      |        else
+      |          y -= 1
+      |      else ()
+      |    }
+      |  }
+      |}""".stripMargin
+
+    checkBCode(code) { dir =>
+      val testClass = loadClassNode(dir.lookupName("Test.class", directory = false).input, skipDebugInfo = false)
+      val testMethod = getMethod(testClass, "test")
+      val lineNumbers = instructionsFromMethod(testMethod).collect{case ln: LineNumber => ln}
+      val expected = List(
+        LineNumber(4, Label(0)),   // var x
+        LineNumber(5, Label(4)),   // var y
+        LineNumber(6, Label(8)),   // while(true)
+        LineNumber(7, Label(13)),  // if (x < y)
+        LineNumber(8, Label(18)),  //   if (x >= y)
+        LineNumber(9, Label(23)),  //     x += 1
+        LineNumber(11, Label(27)), //     y -= 1
+        LineNumber(12, Label(32))  // else ()
+      )
+      assertEquals(expected, lineNumbers)
+    }
+  }
+
+  @Test  def issue18238_c(): Unit = {
+    val code =
+      """
+      |class Test {
+      |  def test(): Unit = {
+      |    var x = 3
+      |    var y = 2
+      |    while(true) {
+      |      if (x < y)
+      |        if (x >= y)
+      |          x += 1
+      |        else
+      |          y -= 1
+      |      println()
+      |    }
+      |  }
+      |}""".stripMargin
+
+    checkBCode(code) { dir =>
+      val testClass = loadClassNode(dir.lookupName("Test.class", directory = false).input, skipDebugInfo = false)
+      val testMethod = getMethod(testClass, "test")
+      val lineNumbers = instructionsFromMethod(testMethod).collect{case ln: LineNumber => ln}
+      val expected = List(
+        LineNumber(4, Label(0)),   // var x
+        LineNumber(5, Label(4)),   // var y
+        LineNumber(6, Label(8)),   // while(true)
+        LineNumber(7, Label(13)),  // if (x < y)
+        LineNumber(8, Label(18)),  //   if (x >= y)
+        LineNumber(9, Label(23)),  //     x += 1
+        LineNumber(11, Label(27)), //     y -= 1
+        LineNumber(12, Label(31))  // println()
+      )
+      assertEquals(expected, lineNumbers)
+    }
+  }