Skip to content

Commit 796a7b9

Browse files
authored
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
2 parents b3f113e + af75f3b commit 796a7b9

File tree

7 files changed

+134
-15
lines changed

7 files changed

+134
-15
lines changed

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

+3-4
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import dotty.tools.dotc.core.Contexts.*
2424
import dotty.tools.dotc.core.Phases.*
2525
import dotty.tools.dotc.core.Decorators.em
2626
import dotty.tools.dotc.report
27+
import dotty.tools.dotc.ast.Trees.SyntheticUnit
2728

2829
/*
2930
*
@@ -218,10 +219,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
218219
val success = new asm.Label
219220
val failure = new asm.Label
220221

221-
val hasElse = !elsep.isEmpty && (elsep match {
222-
case Literal(value) if value.tag == UnitTag => false
223-
case _ => true
224-
})
222+
val hasElse = !elsep.hasAttachment(SyntheticUnit)
225223

226224
genCond(condp, success, failure, targetIfNoJump = success)
227225
markProgramPoint(success)
@@ -250,6 +248,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
250248
if hasElse then
251249
genLoadTo(elsep, expectedType, dest)
252250
else
251+
lineNumber(tree.cond)
253252
genAdaptAndSendToDest(UNIT, expectedType, dest)
254253
expectedType
255254
end if

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ object desugar {
188188
if isSetterNeeded(vdef) then
189189
val setterParam = makeSyntheticParameter(tpt = SetterParamTree().watching(vdef))
190190
// The rhs gets filled in later, when field is generated and getter has parameters (see Memoize miniphase)
191-
val setterRhs = if (vdef.rhs.isEmpty) EmptyTree else unitLiteral
191+
val setterRhs = if (vdef.rhs.isEmpty) EmptyTree else syntheticUnitLiteral
192192
val setter = cpy.DefDef(vdef)(
193193
name = valName.setterName,
194194
paramss = (setterParam :: Nil) :: Nil,
@@ -1489,7 +1489,7 @@ object desugar {
14891489
def block(tree: Block)(using Context): Block = tree.expr match {
14901490
case EmptyTree =>
14911491
cpy.Block(tree)(tree.stats,
1492-
unitLiteral.withSpan(if (tree.stats.isEmpty) tree.span else tree.span.endPos))
1492+
syntheticUnitLiteral.withSpan(if (tree.stats.isEmpty) tree.span else tree.span.endPos))
14931493
case _ =>
14941494
tree
14951495
}
@@ -2017,7 +2017,7 @@ object desugar {
20172017
case ts: Thicket => ts.trees.tail
20182018
case t => Nil
20192019
} map {
2020-
case Block(Nil, EmptyTree) => unitLiteral // for s"... ${} ..."
2020+
case Block(Nil, EmptyTree) => syntheticUnitLiteral // for s"... ${} ..."
20212021
case Block(Nil, expr) => expr // important for interpolated string as patterns, see i1773.scala
20222022
case t => t
20232023
}
@@ -2050,7 +2050,7 @@ object desugar {
20502050
val pats1 = if (tpt.isEmpty) pats else pats map (Typed(_, tpt))
20512051
flatTree(pats1 map (makePatDef(tree, mods, _, rhs)))
20522052
case ext: ExtMethods =>
2053-
Block(List(ext), unitLiteral.withSpan(ext.span))
2053+
Block(List(ext), syntheticUnitLiteral.withSpan(ext.span))
20542054
case f: FunctionWithMods if f.hasErasedParams => makeFunctionWithValDefs(f, pt)
20552055
}
20562056
desugared.withSpan(tree.span)

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,11 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
495495
def InferredTypeTree(tpe: Type)(using Context): TypedSplice =
496496
TypedSplice(new InferredTypeTree().withTypeUnchecked(tpe))
497497

498-
def unitLiteral(implicit src: SourceFile): Literal = Literal(Constant(())).withAttachment(SyntheticUnit, ())
498+
def unitLiteral(implicit src: SourceFile): Literal =
499+
Literal(Constant(()))
500+
501+
def syntheticUnitLiteral(implicit src: SourceFile): Literal =
502+
unitLiteral.withAttachment(SyntheticUnit, ())
499503

500504
def ref(tp: NamedType)(using Context): Tree =
501505
TypedSplice(tpd.ref(tp))

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -1725,7 +1725,7 @@ object Parsers {
17251725
case arg =>
17261726
arg
17271727
val args1 = args.mapConserve(sanitize)
1728-
1728+
17291729
if in.isArrow || isPureArrow || erasedArgs.contains(true) then
17301730
functionRest(args)
17311731
else
@@ -2424,7 +2424,7 @@ object Parsers {
24242424
in.nextToken();
24252425
val expr = subExpr()
24262426
if expr.span.exists then expr
2427-
else unitLiteral // finally without an expression
2427+
else syntheticUnitLiteral // finally without an expression
24282428
}
24292429
else {
24302430
if handler.isEmpty then
@@ -3921,10 +3921,10 @@ object Parsers {
39213921
val stats = selfInvocation() :: (
39223922
if (isStatSep) { in.nextToken(); blockStatSeq() }
39233923
else Nil)
3924-
Block(stats, unitLiteral)
3924+
Block(stats, syntheticUnitLiteral)
39253925
}
39263926
}
3927-
else Block(selfInvocation() :: Nil, unitLiteral)
3927+
else Block(selfInvocation() :: Nil, syntheticUnitLiteral)
39283928

39293929
/** SelfInvocation ::= this ArgumentExprs {ArgumentExprs}
39303930
*/

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

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

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ class ReplCompiler extends Compiler:
159159
def wrap(trees: List[untpd.Tree]): untpd.PackageDef = {
160160
import untpd.*
161161

162-
val valdef = ValDef("expr".toTermName, TypeTree(), Block(trees, unitLiteral).withSpan(Span(0, expr.length)))
162+
val valdef = ValDef("expr".toTermName, TypeTree(), Block(trees, syntheticUnitLiteral).withSpan(Span(0, expr.length)))
163163
val tmpl = Template(emptyConstructor, Nil, Nil, EmptyValDef, List(valdef))
164164
val wrapper = TypeDef("$wrapper".toTypeName, tmpl)
165165
.withMods(Modifiers(Final))
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
package dotty.tools.backend.jvm
2+
3+
import scala.language.unsafeNulls
4+
5+
import org.junit.Assert._
6+
import org.junit.Test
7+
8+
class SourcePositionsTest extends DottyBytecodeTest:
9+
import ASMConverters._
10+
11+
@Test def issue18238_a(): Unit = {
12+
val code =
13+
"""
14+
|class Test {
15+
| def test(): Unit = {
16+
| var x = 3
17+
| var y = 2
18+
| while(true) {
19+
| if (x < y)
20+
| if (x >= y)
21+
| x += 1
22+
| else
23+
| y -= 1
24+
| }
25+
| }
26+
|}""".stripMargin
27+
28+
checkBCode(code) { dir =>
29+
val testClass = loadClassNode(dir.lookupName("Test.class", directory = false).input, skipDebugInfo = false)
30+
val testMethod = getMethod(testClass, "test")
31+
val lineNumbers = instructionsFromMethod(testMethod).collect{case ln: LineNumber => ln}
32+
val expected = List(
33+
LineNumber(4, Label(0)), // var x
34+
LineNumber(5, Label(4)), // var y
35+
LineNumber(6, Label(8)), // while(true)
36+
LineNumber(7, Label(13)), // if (x < y)
37+
LineNumber(8, Label(18)), // if (x >= y)
38+
LineNumber(9, Label(23)), // x += 1
39+
LineNumber(11, Label(27)), // y -= 1
40+
LineNumber(7, Label(32)) // <synthetic unit> point back to `if (x < y)
41+
)
42+
assertEquals(expected, lineNumbers)
43+
}
44+
}
45+
46+
@Test def issue18238_b(): Unit = {
47+
val code =
48+
"""
49+
|class Test {
50+
| def test(): Unit = {
51+
| var x = 3
52+
| var y = 2
53+
| while(true) {
54+
| if (x < y)
55+
| if (x >= y)
56+
| x += 1
57+
| else
58+
| y -= 1
59+
| else ()
60+
| }
61+
| }
62+
|}""".stripMargin
63+
64+
checkBCode(code) { dir =>
65+
val testClass = loadClassNode(dir.lookupName("Test.class", directory = false).input, skipDebugInfo = false)
66+
val testMethod = getMethod(testClass, "test")
67+
val lineNumbers = instructionsFromMethod(testMethod).collect{case ln: LineNumber => ln}
68+
val expected = List(
69+
LineNumber(4, Label(0)), // var x
70+
LineNumber(5, Label(4)), // var y
71+
LineNumber(6, Label(8)), // while(true)
72+
LineNumber(7, Label(13)), // if (x < y)
73+
LineNumber(8, Label(18)), // if (x >= y)
74+
LineNumber(9, Label(23)), // x += 1
75+
LineNumber(11, Label(27)), // y -= 1
76+
LineNumber(12, Label(32)) // else ()
77+
)
78+
assertEquals(expected, lineNumbers)
79+
}
80+
}
81+
82+
@Test def issue18238_c(): Unit = {
83+
val code =
84+
"""
85+
|class Test {
86+
| def test(): Unit = {
87+
| var x = 3
88+
| var y = 2
89+
| while(true) {
90+
| if (x < y)
91+
| if (x >= y)
92+
| x += 1
93+
| else
94+
| y -= 1
95+
| println()
96+
| }
97+
| }
98+
|}""".stripMargin
99+
100+
checkBCode(code) { dir =>
101+
val testClass = loadClassNode(dir.lookupName("Test.class", directory = false).input, skipDebugInfo = false)
102+
val testMethod = getMethod(testClass, "test")
103+
val lineNumbers = instructionsFromMethod(testMethod).collect{case ln: LineNumber => ln}
104+
val expected = List(
105+
LineNumber(4, Label(0)), // var x
106+
LineNumber(5, Label(4)), // var y
107+
LineNumber(6, Label(8)), // while(true)
108+
LineNumber(7, Label(13)), // if (x < y)
109+
LineNumber(8, Label(18)), // if (x >= y)
110+
LineNumber(9, Label(23)), // x += 1
111+
LineNumber(11, Label(27)), // y -= 1
112+
LineNumber(12, Label(31)) // println()
113+
)
114+
assertEquals(expected, lineNumbers)
115+
}
116+
}

0 commit comments

Comments
 (0)