Skip to content

Commit ee4a6ce

Browse files
TheElectronWillTheElectronWill
committed
Fix liftForCoverage, in particular for erased arguments
Fixes #15505
1 parent 6e7adfc commit ee4a6ce

File tree

6 files changed

+280
-25
lines changed

6 files changed

+280
-25
lines changed

‎compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package transform
44
import java.io.File
55
import java.util.concurrent.atomic.AtomicInteger
66

7+
import ast.tpd.*
78
import collection.mutable
89
import core.Flags.*
910
import core.Contexts.{Context, ctx, inContext}
@@ -23,7 +24,6 @@ import coverage.*
2324
* The result can then be consumed by the Scoverage tool.
2425
*/
2526
class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
26-
import ast.tpd._
2727

2828
override def phaseName = InstrumentCoverage.name
2929

@@ -60,7 +60,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
6060

6161
/** Transforms trees to insert calls to Invoker.invoked to compute the coverage when the code is called */
6262
private class CoverageTransformer extends Transformer:
63-
override def transform(tree: Tree)(using ctx: Context): Tree =
63+
override def transform(tree: Tree)(using Context): Tree =
6464
inContext(transformCtx(tree)) { // necessary to position inlined code properly
6565
tree match
6666
// simple cases
@@ -280,10 +280,6 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
280280
sym.exists &&
281281
sym == defn.Boolean_&& || sym == defn.Boolean_||
282282

283-
def isContextual(fun: Apply): Boolean =
284-
val args = fun.args
285-
args.nonEmpty && args.head.symbol.isAllOf(Given | Implicit)
286-
287283
val fun = tree.fun
288284
val nestedApplyNeedsLift = fun match
289285
case a: Apply => needsLift(a)

‎compiler/src/dotty/tools/dotc/typer/EtaExpansion.scala‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -159,23 +159,39 @@ class LiftComplex extends Lifter {
159159
}
160160
object LiftComplex extends LiftComplex
161161

162-
/** Lift complex + lift the prefixes */
163-
object LiftCoverage extends LiftComplex {
162+
/** Lift impure + lift the prefixes */
163+
object LiftCoverage extends LiftImpure {
164164

165-
private val LiftEverything = new Property.Key[Boolean]
165+
// Property indicating whether we're currently lifting the arguments of an application
166+
private val LiftingArgs = new Property.Key[Boolean]
166167

167-
private inline def liftEverything(using Context): Boolean =
168-
ctx.property(LiftEverything).contains(true)
168+
private inline def liftingArgs(using Context): Boolean =
169+
ctx.property(LiftingArgs).contains(true)
169170

170-
private def liftEverythingContext(using Context): Context =
171-
ctx.fresh.setProperty(LiftEverything, true)
171+
private def liftingArgsContext(using Context): Context =
172+
ctx.fresh.setProperty(LiftingArgs, true)
173+
174+
/** Variant of `noLift` for the arguments of applications.
175+
* To produce the right coverage information (especially in case of exceptions), we must lift:
176+
* - all the applications, except the erased ones
177+
* - all the impure arguments
178+
*
179+
* There's no need to lift the other arguments.
180+
*/
181+
private def noLiftArg(arg: tpd.Tree)(using Context): Boolean =
182+
arg match
183+
case a: tpd.Apply => a.symbol.is(Erased) // don't lift erased applications, but lift all others
184+
case tpd.Block(stats, expr) => stats.forall(noLiftArg) && noLiftArg(expr)
185+
case tpd.Inlined(_, bindings, expr) => noLiftArg(expr)
186+
case tpd.Typed(expr, _) => noLiftArg(expr)
187+
case _ => super.noLift(arg)
172188

173189
override def noLift(expr: tpd.Tree)(using Context) =
174-
!liftEverything && super.noLift(expr)
190+
if liftingArgs then noLiftArg(expr) else super.noLift(expr)
175191

176192
def liftForCoverage(defs: mutable.ListBuffer[tpd.Tree], tree: tpd.Apply)(using Context) = {
177193
val liftedFun = liftApp(defs, tree.fun)
178-
val liftedArgs = liftArgs(defs, tree.fun.tpe, tree.args)(using liftEverythingContext)
194+
val liftedArgs = liftArgs(defs, tree.fun.tpe, tree.args)(using liftingArgsContext)
179195
tpd.cpy.Apply(tree)(liftedFun, liftedArgs)
180196
}
181197
}

‎tests/coverage/pos/ContextFunctions.scoverage.check‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,16 +92,16 @@ covtest
9292
Imperative
9393
Class
9494
covtest.Imperative
95-
$anonfun
96-
267
97-
294
95+
readPerson
96+
252
97+
295
9898
13
9999
invoked
100100
Apply
101101
false
102102
0
103103
false
104-
readName2(using e)(using s)
104+
OnError((e) => readName2(using e)(using s))
105105

106106
5
107107
ContextFunctions.scala
@@ -113,7 +113,7 @@ $anonfun
113113
267
114114
294
115115
13
116-
apply
116+
invoked
117117
Apply
118118
false
119119
0
@@ -126,16 +126,16 @@ covtest
126126
Imperative
127127
Class
128128
covtest.Imperative
129-
readPerson
130-
252
131-
295
129+
$anonfun
130+
267
131+
294
132132
13
133-
invoked
133+
apply
134134
Apply
135135
false
136136
0
137137
false
138-
OnError((e) => readName2(using e)(using s))
138+
readName2(using e)(using s)
139139

140140
7
141141
ContextFunctions.scala

‎tests/coverage/run/erased/test.check‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
foo(a)(b)
2+
identity(idem)
3+
foo(a)(idem)

‎tests/coverage/run/erased/test.scala‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import scala.language.experimental.erasedDefinitions
2+
3+
erased def e(x: String): String = "x"
4+
def foo(erased a: String)(b: String): String =
5+
println(s"foo(a)($b)")
6+
b
7+
8+
def identity(s: String): String =
9+
println(s"identity($s)")
10+
s
11+
12+
@main
13+
def Test: Unit =
14+
foo(e("a"))("b")
15+
foo(e("a"))(identity("idem"))

‎tests/coverage/run/erased/test.scoverage.check‎

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
# Coverage data, format version: 3.0
2+
# Statement data:
3+
# - id
4+
# - source path
5+
# - package name
6+
# - class name
7+
# - class type (Class, Object or Trait)
8+
# - full class name
9+
# - method name
10+
# - start offset
11+
# - end offset
12+
# - line number
13+
# - symbol name
14+
# - tree name
15+
# - is branch
16+
# - invocations count
17+
# - is ignored
18+
# - description (can be multi-line)
19+
# ' ' sign
20+
# ------------------------------------------
21+
0
22+
erased/test.scala
23+
<empty>
24+
test$package$
25+
Object
26+
<empty>.test$package$
27+
e
28+
54
29+
66
30+
2
31+
e
32+
DefDef
33+
false
34+
0
35+
false
36+
erased def e
37+
38+
1
39+
erased/test.scala
40+
<empty>
41+
test$package$
42+
Object
43+
<empty>.test$package$
44+
foo
45+
149
46+
162
47+
4
48+
s
49+
Apply
50+
false
51+
0
52+
false
53+
s"foo(a)($b)"
54+
55+
2
56+
erased/test.scala
57+
<empty>
58+
test$package$
59+
Object
60+
<empty>.test$package$
61+
foo
62+
141
63+
163
64+
4
65+
println
66+
Apply
67+
false
68+
0
69+
false
70+
println(s"foo(a)($b)")
71+
72+
3
73+
erased/test.scala
74+
<empty>
75+
test$package$
76+
Object
77+
<empty>.test$package$
78+
foo
79+
92
80+
99
81+
3
82+
foo
83+
DefDef
84+
false
85+
0
86+
false
87+
def foo
88+
89+
4
90+
erased/test.scala
91+
<empty>
92+
test$package$
93+
Object
94+
<empty>.test$package$
95+
identity
96+
213
97+
228
98+
8
99+
s
100+
Apply
101+
false
102+
0
103+
false
104+
s"identity($s)"
105+
106+
5
107+
erased/test.scala
108+
<empty>
109+
test$package$
110+
Object
111+
<empty>.test$package$
112+
identity
113+
205
114+
229
115+
8
116+
println
117+
Apply
118+
false
119+
0
120+
false
121+
println(s"identity($s)")
122+
123+
6
124+
erased/test.scala
125+
<empty>
126+
test$package$
127+
Object
128+
<empty>.test$package$
129+
identity
130+
169
131+
181
132+
7
133+
identity
134+
DefDef
135+
false
136+
0
137+
false
138+
def identity
139+
140+
7
141+
erased/test.scala
142+
<empty>
143+
test$package$
144+
Object
145+
<empty>.test$package$
146+
Test
147+
264
148+
270
149+
13
150+
e
151+
Apply
152+
false
153+
0
154+
false
155+
e("a")
156+
157+
8
158+
erased/test.scala
159+
<empty>
160+
test$package$
161+
Object
162+
<empty>.test$package$
163+
Test
164+
260
165+
276
166+
13
167+
foo
168+
Apply
169+
false
170+
0
171+
false
172+
foo(e("a"))("b")
173+
174+
9
175+
erased/test.scala
176+
<empty>
177+
test$package$
178+
Object
179+
<empty>.test$package$
180+
Test
181+
291
182+
307
183+
14
184+
identity
185+
Apply
186+
false
187+
0
188+
false
189+
identity("idem")
190+
191+
10
192+
erased/test.scala
193+
<empty>
194+
test$package$
195+
Object
196+
<empty>.test$package$
197+
Test
198+
279
199+
308
200+
14
201+
foo
202+
Apply
203+
false
204+
0
205+
false
206+
foo(e("a"))(identity("idem"))
207+
208+
11
209+
erased/test.scala
210+
<empty>
211+
test$package$
212+
Object
213+
<empty>.test$package$
214+
Test
215+
235
216+
249
217+
12
218+
Test
219+
DefDef
220+
false
221+
0
222+
false
223+
@main
224+
def Test
225+

0 commit comments

Comments
 (0)