From 4d8bf775893a020d10042a1e326d293d4bea6f57 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 --- 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 db52a74300ef..46a62c73aa77 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 4afb8c79de92463fda855520c11593fd276657b5 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 --- 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 46a62c73aa77..565ad72c0d9d 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 8c52866c98823bb9f4030ef3ffac8cc32e1426cb Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Sat, 18 May 2024 00:05:31 +0200 Subject: [PATCH 3/4] Introduce `untpd.syntheticUnitLiteral` to allow detection of explicit unit provided by user from synthetic unit introduced by compiler --- 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 | 8 ++++---- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- compiler/src/dotty/tools/repl/ReplCompiler.scala | 2 +- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index b1b771bc7512..b1c70d0d3d36 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/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 } @@ -2013,7 +2013,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 } @@ -2046,7 +2046,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 64f9fb4df95e..c42e8f71246d 100644 --- a/compiler/src/dotty/tools/dotc/ast/untpd.scala +++ b/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)) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index e28ba5fd669e..4c13934f3473 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/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} */ diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index ae50d626cb1f..bccf7b952b0c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2221,7 +2221,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 d69173cb6d88..f909abfc129a 100644 --- a/compiler/src/dotty/tools/repl/ReplCompiler.scala +++ b/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)) From af75f3b266f8ddca74e99c73c67e59995c4729e3 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 --- .../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) + } + }