Don't emit line number for synthetic unit value

lrytz · WojciechMazur · commit 240b1aefc87e · 2024-06-21T19:43:51.000+02:00
Synthetic `()` values are added to blocks without an expression. Don't emit a line number for them. Implemented by checking the `SyntheticUnit` attachment. This seems simpler than trying to control the position assigned to synthetic unit trees, as they are created in many places. [Cherry-picked e204e30]
diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala
@@ -3,26 +3,24 @@ package backend
 package jvm
 
 import scala.language.unsafeNulls
-
 import scala.annotation.tailrec
-
-import scala.collection.{ mutable, immutable }
-
+import scala.collection.{immutable, mutable}
 import scala.tools.asm
 import dotty.tools.dotc.ast.tpd
 import dotty.tools.dotc.ast.TreeTypeMap
 import dotty.tools.dotc.CompilationUnit
-import dotty.tools.dotc.core.Decorators._
-import dotty.tools.dotc.core.Flags._
-import dotty.tools.dotc.core.StdNames._
-import dotty.tools.dotc.core.NameKinds._
+import dotty.tools.dotc.ast.Trees.SyntheticUnit
+import dotty.tools.dotc.core.Decorators.*
+import dotty.tools.dotc.core.Flags.*
+import dotty.tools.dotc.core.StdNames.*
+import dotty.tools.dotc.core.NameKinds.*
 import dotty.tools.dotc.core.Names.TermName
-import dotty.tools.dotc.core.Symbols._
-import dotty.tools.dotc.core.Types._
-import dotty.tools.dotc.core.Contexts._
-import dotty.tools.dotc.util.Spans._
+import dotty.tools.dotc.core.Symbols.*
+import dotty.tools.dotc.core.Types.*
+import dotty.tools.dotc.core.Contexts.*
+import dotty.tools.dotc.util.Spans.*
 import dotty.tools.dotc.report
-import dotty.tools.dotc.transform.SymUtils._
+import dotty.tools.dotc.transform.SymUtils.*
 
 /*
  *
@@ -624,16 +622,17 @@ trait BCodeSkelBuilder extends BCodeHelpers {
         case _                     => a
       }
 
-      if (!emitLines || !tree.span.exists) return;
-      val nr = ctx.source.offsetToLine(tree.span.point) + 1
-      if (nr != lastEmittedLineNr) {
-        lastEmittedLineNr = nr
-        getNonLabelNode(lastInsn) match {
-          case lnn: asm.tree.LineNumberNode =>
-            // overwrite previous landmark as no instructions have been emitted for it
-            lnn.line = nr
-          case _ =>
-            mnode.visitLineNumber(nr, currProgramPoint())
+      if (emitLines && tree.span.exists && !tree.hasAttachment(SyntheticUnit)) {
+        val nr = ctx.source.offsetToLine(tree.span.point) + 1
+        if (nr != lastEmittedLineNr) {
+          lastEmittedLineNr = nr
+          getNonLabelNode(lastInsn) match {
+            case lnn: asm.tree.LineNumberNode =>
+              // overwrite previous landmark as no instructions have been emitted for it
+              lnn.line = nr
+            case _ =>
+              mnode.visitLineNumber(nr, currProgramPoint())
+          }
         }
       }
     }
diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
@@ -1809,7 +1809,7 @@ object desugar {
           case ts: Thicket => ts.trees.tail
           case t => Nil
         } map {
-          case Block(Nil, EmptyTree) => Literal(Constant(())) // for s"... ${} ..."
+          case Block(Nil, EmptyTree) => unitLiteral // for s"... ${} ..."
           case Block(Nil, expr) => expr // important for interpolated string as patterns, see i1773.scala
           case t => t
         }
@@ -1837,7 +1837,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), Literal(Constant(())).withSpan(ext.span))
+        Block(List(ext), unitLiteral.withSpan(ext.span))
       case f: FunctionWithMods if f.hasErasedParams => makeFunctionWithValDefs(f, pt)
     }
     desugared.withSpan(tree.span)
diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala
@@ -31,6 +31,8 @@ object Trees {
 
   /** Property key for backquoted identifiers and definitions */
   val Backquoted: Property.StickyKey[Unit] = Property.StickyKey()
+  
+  val SyntheticUnit: Property.StickyKey[Unit] = Property.StickyKey()
 
   /** Trees take a parameter indicating what the type of their `tpe` field
    *  is. Two choices: `Type` or `Untyped`.
diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala
@@ -64,7 +64,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
     ta.assignType(untpd.Literal(const))
 
   def unitLiteral(using Context): Literal =
-    Literal(Constant(()))
+    Literal(Constant(())).withAttachment(SyntheticUnit, ())
 
   def nullLiteral(using Context): Literal =
     Literal(Constant(null))
diff --git a/compiler/src/dotty/tools/dotc/ast/untpd.scala b/compiler/src/dotty/tools/dotc/ast/untpd.scala
@@ -479,7 +479,7 @@ 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(()))
+  def unitLiteral(implicit src: SourceFile): Literal = Literal(Constant(())).withAttachment(SyntheticUnit, ())
 
   def ref(tp: NamedType)(using Context): Tree =
     TypedSplice(tpd.ref(tp))
diff --git a/compiler/src/dotty/tools/dotc/inlines/Inliner.scala b/compiler/src/dotty/tools/dotc/inlines/Inliner.scala
@@ -793,7 +793,7 @@ class Inliner(val call: tpd.Tree)(using Context):
       typed(tree.cond, defn.BooleanType)(using condCtx) match {
         case cond1 @ ConstantValue(b: Boolean) =>
           val selected0 = if (b) tree.thenp else tree.elsep
-          val selected = if (selected0.isEmpty) tpd.Literal(Constant(())) else typed(selected0, pt)
+          val selected = if (selected0.isEmpty) tpd.unitLiteral else typed(selected0, pt)
           if (isIdempotentExpr(cond1)) selected
           else Block(cond1 :: Nil, selected)
         case cond1 =>
diff --git a/compiler/src/dotty/tools/dotc/inlines/Inlines.scala b/compiler/src/dotty/tools/dotc/inlines/Inlines.scala
@@ -408,7 +408,7 @@ object Inlines:
             arg match
               case ConstantValue(_) | Inlined(_, Nil, Typed(ConstantValue(_), _)) => // ok
               case _ => report.error(em"expected a constant value but found: $arg", arg.srcPos)
-            return Literal(Constant(())).withSpan(call.span)
+            return unitLiteral.withSpan(call.span)
           else if inlinedMethod == defn.Compiletime_codeOf then
             return Intrinsics.codeOf(arg, call.srcPos)
         case _ =>
diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
@@ -2180,7 +2180,7 @@ object Parsers {
               case _ =>
             }
             patch(source, cond.span.endPos, "}) ()")
-          WhileDo(Block(body, cond), Literal(Constant(())))
+          WhileDo(Block(body, cond), unitLiteral)
         }
       case TRY =>
         val tryOffset = in.offset
@@ -2210,7 +2210,7 @@ object Parsers {
               in.nextToken();
               val expr = subExpr()
               if expr.span.exists then expr
-              else Literal(Constant(())) // finally without an expression
+              else unitLiteral // finally without an expression
             }
             else {
               if handler.isEmpty then
@@ -3701,10 +3701,10 @@ object Parsers {
             val stats = selfInvocation() :: (
               if (isStatSep) { in.nextToken(); blockStatSeq() }
               else Nil)
-            Block(stats, Literal(Constant(())))
+            Block(stats, unitLiteral)
           }
         }
-      else Block(selfInvocation() :: Nil, Literal(Constant(())))
+      else Block(selfInvocation() :: Nil, unitLiteral)
 
     /** SelfInvocation  ::= this ArgumentExprs {ArgumentExprs}
      */
diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala
@@ -122,7 +122,7 @@ class DropBreaks extends MiniPhase:
           case id: Ident =>
             val arg = (args: @unchecked) match
               case arg :: Nil => arg
-              case Nil => Literal(Constant(())).withSpan(tree.span)
+              case Nil => unitLiteral.withSpan(tree.span)
             Some((id.symbol, arg))
           case _ => None
       case _ => None
diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala
@@ -317,7 +317,7 @@ object Erasure {
           cast(tree1, pt)
         case _ =>
           val cls = pt.classSymbol
-          if (cls eq defn.UnitClass) constant(tree, Literal(Constant(())))
+          if (cls eq defn.UnitClass) constant(tree, unitLiteral)
           else {
             assert(cls ne defn.ArrayClass)
             ref(unboxMethod(cls.asClass)).appliedTo(tree)
diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala
@@ -173,7 +173,7 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
             myState.classesThatNeedReleaseFence += sym.owner
           val initializer =
             if isErasableBottomField(field, tree.termParamss.head.head.tpt.tpe.classSymbol)
-            then Literal(Constant(()))
+            then unitLiteral
             else Assign(ref(field), adaptToField(field, ref(tree.termParamss.head.head.symbol)))
           val setterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(using ctx.withOwner(sym)))
           sym.keepAnnotationsCarrying(thisPhase, Set(defn.SetterMetaAnnot))
diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala
@@ -947,7 +947,7 @@ object PatternMatcher {
         case LabeledPlan(label, expr) =>
           Labeled(label, emit(expr))
         case ReturnPlan(label) =>
-          Return(Literal(Constant(())), ref(label))
+          Return(unitLiteral, ref(label))
         case plan: SeqPlan =>
           def default = seq(emit(plan.head) :: Nil, emit(plan.tail))
           def maybeEmitSwitch(scrutinee: Tree): Tree = {
diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
@@ -285,7 +285,7 @@ object TypeTestsCasts {
             Typed(expr, tree.args.head) // Replace cast by type ascription (which does not generate any bytecode)
           else if (testCls eq defn.BoxedUnitClass)
             // as a special case, casting to Unit always successfully returns Unit
-            Block(expr :: Nil, Literal(Constant(()))).withSpan(expr.span)
+            Block(expr :: Nil, unitLiteral).withSpan(expr.span)
           else if (foundClsSymPrimitive)
             if (testCls.isPrimitiveValueClass) primitiveConversion(expr, testCls)
             else derivedTree(box(expr), defn.Any_asInstanceOf, testType)
diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala
@@ -1547,7 +1547,7 @@ class RefChecks extends MiniPhase { thisPhase =>
     private def transformIf(tree: If): Tree = {
       val If(cond, thenpart, elsepart) = tree
       def unitIfEmpty(t: Tree): Tree =
-        if (t == EmptyTree) Literal(Constant(())).setPos(tree.pos).setType(UnitTpe) else t
+        if (t == EmptyTree) unitLiteral.setPos(tree.pos).setType(UnitTpe) else t
 
       cond.tpe match {
         case ConstantType(value) =>
diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -4117,7 +4117,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value && !isThisTypeResult(tree)) {
           report.warning(ValueDiscarding(tree.tpe), tree.srcPos)
         }
-        return tpd.Block(tree1 :: Nil, Literal(Constant(())))
+        return tpd.Block(tree1 :: Nil, unitLiteral)
       }
 
       // convert function literal to SAM closure
diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala
@@ -1682,6 +1682,25 @@ class DottyBytecodeTests extends DottyBytecodeTest {
       assertSameCode(instructions, expected)
     }
   }
+
+  @Test def i18320(): Unit = {
+    val c1 =
+      """class C {
+        |  def m: Unit = {
+        |    val x = 1
+        |  }
+        |}
+        |""".stripMargin
+    checkBCode(c1) {dir =>
+      val clsIn = dir.lookupName("C.class", directory = false).input
+      val clsNode = loadClassNode(clsIn, skipDebugInfo = false)
+      val method = getMethod(clsNode, "m")
+      val instructions = instructionsFromMethod(method).filter(_.isInstanceOf[LineNumber])
+      val expected = List(LineNumber(3, Label(0)))
+      assertSameCode(instructions, expected)
+
+    }
+  }
 }
 
 object invocationReceiversTestCode {

Original file line number	Diff line number	Diff line change
`@@ -2180,7 +2180,7 @@ object Parsers {`
`2180`	`2180`	`case _ =>`
`2181`	`2181`	`}`
`2182`	`2182`	`patch(source, cond.span.endPos, "}) ()")`
`2183`		`- WhileDo(Block(body, cond), Literal(Constant(())))`
	`2183`	`+ WhileDo(Block(body, cond), unitLiteral)`
`2184`	`2184`	`}`
`2185`	`2185`	`case TRY =>`
`2186`	`2186`	`val tryOffset = in.offset`
`@@ -2210,7 +2210,7 @@ object Parsers {`
`2210`	`2210`	`in.nextToken();`
`2211`	`2211`	`val expr = subExpr()`
`2212`	`2212`	`if expr.span.exists then expr`
`2213`		`- else Literal(Constant(())) // finally without an expression`
	`2213`	`+ else unitLiteral // finally without an expression`
`2214`	`2214`	`}`
`2215`	`2215`	`else {`
`2216`	`2216`	`if handler.isEmpty then`
`@@ -3701,10 +3701,10 @@ object Parsers {`
`3701`	`3701`	`val stats = selfInvocation() :: (`
`3702`	`3702`	`if (isStatSep) { in.nextToken(); blockStatSeq() }`
`3703`	`3703`	`else Nil)`
`3704`		`- Block(stats, Literal(Constant(())))`
	`3704`	`+ Block(stats, unitLiteral)`
`3705`	`3705`	`}`
`3706`	`3706`	`}`
`3707`		`- else Block(selfInvocation() :: Nil, Literal(Constant(())))`
	`3707`	`+ else Block(selfInvocation() :: Nil, unitLiteral)`
`3708`	`3708`
`3709`	`3709`	`/** SelfInvocation ::= this ArgumentExprs {ArgumentExprs}`
`3710`	`3710`	`*/`