From 57c972047cf4ebffed4122ea3764473945930a40 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Sat, 18 May 2024 00:00:35 +0200 Subject: [PATCH 1/4] Use SyntheticUnit attachment to detect if has else branch [Cherry-picked 4d8bf775893a020d10042a1e326d293d4bea6f57] --- compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index d7c84f98f49a..bcfcedfeb672 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/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) From 281c9b7ff77f2d0690f40071a0319b958fb736e3 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Sat, 18 May 2024 00:03:33 +0200 Subject: [PATCH 2/4] Emit explicit line position for synthetic unit pointing to if condition line [Cherry-picked 4afb8c79de92463fda855520c11593fd276657b5] --- compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index bcfcedfeb672..25949620a124 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -248,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 From e9a62347238af5567bb853829040032b3fd49be2 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Tue, 9 Jul 2024 23:50:58 +0200 Subject: [PATCH 3/4] Introduce `untpd.syntheticUnitLiteral` to allow detection of explicit unit provided by user from synthetic unit introduced by compiler [Cherry-picked 8c52866c98823bb9f4030ef3ffac8cc32e1426cb][modified] --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 8 ++++---- compiler/src/dotty/tools/dotc/ast/untpd.scala | 6 +++++- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 6 +++--- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- compiler/src/dotty/tools/repl/ReplCompiler.scala | 2 +- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 80e69c4ed8b1..6863618bacfc 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -190,7 +190,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, @@ -1311,7 +1311,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 } @@ -1811,7 +1811,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 } @@ -1839,7 +1839,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) diff --git a/compiler/src/dotty/tools/dotc/ast/untpd.scala b/compiler/src/dotty/tools/dotc/ast/untpd.scala index 32a09b6e0db1..4c8fea17c076 100644 --- a/compiler/src/dotty/tools/dotc/ast/untpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/untpd.scala @@ -481,7 +481,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)) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index a6521bb870b8..3bd458979754 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -2284,7 +2284,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 @@ -3768,10 +3768,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} */ diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index d8f427ff9206..027c57d8662b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2009,7 +2009,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 diff --git a/compiler/src/dotty/tools/repl/ReplCompiler.scala b/compiler/src/dotty/tools/repl/ReplCompiler.scala index af3fb32c3e86..5c8dcc2616b7 100644 --- a/compiler/src/dotty/tools/repl/ReplCompiler.scala +++ b/compiler/src/dotty/tools/repl/ReplCompiler.scala @@ -158,7 +158,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)) From e680362c4d25a600f461bb8c9729261f810fa9ab Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Sat, 18 May 2024 00:06:31 +0200 Subject: [PATCH 4/4] Add unit tests for issue 18238 - ensure we generate correct line positions for if conditions [Cherry-picked af75f3b266f8ddca74e99c73c67e59995c4729e3] --- .../backend/jvm/SourcePositionsTest.scala | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 compiler/test/dotty/tools/backend/jvm/SourcePositionsTest.scala diff --git a/compiler/test/dotty/tools/backend/jvm/SourcePositionsTest.scala b/compiler/test/dotty/tools/backend/jvm/SourcePositionsTest.scala new file mode 100644 index 000000000000..7bb52260c366 --- /dev/null +++ b/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)) // 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) + } + }