Skip to content

Commit c535dbc

Browse files
authored
Make coverage more similar to the one in Scala 2 (#23722)
Closes #21877 * removes coverage of inlined nodes (as mentioned in the accompanying comment, those are impossible to represent in most cases) * adds coverage for Literals (ones directly in Apply are omitted) * removes coverage of `throw` contents * if apply node is tagged, we do not tag it's prefix, outside of other prefixing Apply's arguments (eg. when we tag `a+b+c` we do not redundantly tag `a+b`) * allows instrumenting synthetic method calls (like apply of a case After all of these changes the statements tagged are much more similar to Scala 2, let's look at the #21877 minimisation: * Scala 2: <img width="704" height="364" alt="Zrzut ekranu 2025-08-12 o 17 07 31" src="https://github.com/user-attachments/assets/f647dfa5-973e-424f-9818-483b7d01d550" /> <img width="740" height="379" alt="Zrzut ekranu 2025-08-12 o 17 07 46" src="https://github.com/user-attachments/assets/09eca1c0-a202-4e5e-b3e4-0947d9e8662d" /> * Scala 3: <img width="623" height="360" alt="Zrzut ekranu 2025-08-12 o 17 08 48" src="https://github.com/user-attachments/assets/efd5baaa-9f52-4ad6-9ba6-2f5bde42a470" /> <img width="638" height="428" alt="Zrzut ekranu 2025-08-12 o 17 08 55" src="https://github.com/user-attachments/assets/01ff6cc6-c348-47db-8ae5-d758ca0bf302" /> There are some differences still remaining, most notably the tagging the DefDefs and its default parameters, but I left them for now, as those seem more useful than harmful. BEcouse of those changed most of the .covergae files had to be regenerated, however I want through each and every diff to make sure that all of those changes there are expected. Additionally, this PR also fixes #21695 (issue with certain generated Block nodes not having assigned the correct type, causing later undefined errors).
1 parent 0cf7a18 commit c535dbc

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+2819
-1202
lines changed

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

Lines changed: 56 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,31 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
145145
val span = pos.span.toSynthetic
146146
invokeCall(statementId, span)
147147

148+
private def transformApplyArgs(trees: List[Tree])(using Context): List[Tree] =
149+
if allConstArgs(trees) then trees else transform(trees)
150+
151+
private def transformInnerApply(tree: Tree)(using Context): Tree = tree match
152+
case a: Apply if a.fun.symbol == defn.StringContextModule_apply =>
153+
a
154+
case a: Apply =>
155+
cpy.Apply(a)(
156+
transformInnerApply(a.fun),
157+
transformApplyArgs(a.args)
158+
)
159+
case a: TypeApply =>
160+
cpy.TypeApply(a)(
161+
transformInnerApply(a.fun),
162+
transformApplyArgs(a.args)
163+
)
164+
case s: Select =>
165+
cpy.Select(s)(transformInnerApply(s.qualifier), s.name)
166+
case i: (Ident | This) => i
167+
case t: Typed =>
168+
cpy.Typed(t)(transformInnerApply(t.expr), t.tpt)
169+
case other => transform(other)
170+
171+
private def allConstArgs(args: List[Tree]) =
172+
args.forall(arg => arg.isInstanceOf[Literal] || arg.isInstanceOf[Ident])
148173
/**
149174
* Tries to instrument an `Apply`.
150175
* These "tryInstrument" methods are useful to tweak the generation of coverage instrumentation,
@@ -158,10 +183,12 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
158183
// Create a call to Invoker.invoked(coverageDirectory, newStatementId)
159184
val coverageCall = createInvokeCall(tree, tree.sourcePos)
160185

161-
if needsLift(tree) then
162-
// Transform args and fun, i.e. instrument them if needed (and if possible)
163-
val app = cpy.Apply(tree)(transform(tree.fun), tree.args.map(transform))
186+
// Transform args and fun, i.e. instrument them if needed (and if possible)
187+
val app =
188+
if tree.fun.symbol eq defn.throwMethod then tree
189+
else cpy.Apply(tree)(transformInnerApply(tree.fun), transformApplyArgs(tree.args))
164190

191+
if needsLift(tree) then
165192
// Lifts the arguments. Note that if only one argument needs to be lifted, we lift them all.
166193
// Also, tree.fun can be lifted too.
167194
// See LiftCoverage for the internal working of this lifting.
@@ -171,11 +198,10 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
171198
InstrumentedParts(liftedDefs.toList, coverageCall, liftedApp)
172199
else
173200
// Instrument without lifting
174-
val transformed = cpy.Apply(tree)(transform(tree.fun), transform(tree.args))
175-
InstrumentedParts.singleExpr(coverageCall, transformed)
201+
InstrumentedParts.singleExpr(coverageCall, app)
176202
else
177203
// Transform recursively but don't instrument the tree itself
178-
val transformed = cpy.Apply(tree)(transform(tree.fun), transform(tree.args))
204+
val transformed = cpy.Apply(tree)(transformInnerApply(tree.fun), transform(tree.args))
179205
InstrumentedParts.notCovered(transformed)
180206

181207
private def tryInstrument(tree: Ident)(using Context): InstrumentedParts =
@@ -187,9 +213,14 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
187213
else
188214
InstrumentedParts.notCovered(tree)
189215

216+
private def tryInstrument(tree: Literal)(using Context): InstrumentedParts =
217+
val coverageCall = createInvokeCall(tree, tree.sourcePos)
218+
InstrumentedParts.singleExpr(coverageCall, tree)
219+
190220
private def tryInstrument(tree: Select)(using Context): InstrumentedParts =
191221
val sym = tree.symbol
192-
val transformed = cpy.Select(tree)(transform(tree.qualifier), tree.name)
222+
val qual = transform(tree.qualifier).ensureConforms(tree.qualifier.tpe)
223+
val transformed = cpy.Select(tree)(qual, tree.name)
193224
if canInstrumentParameterless(sym) then
194225
// call to a parameterless method
195226
val coverageCall = createInvokeCall(tree, tree.sourcePos)
@@ -202,6 +233,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
202233
tree match
203234
case t: Apply => tryInstrument(t)
204235
case t: Ident => tryInstrument(t)
236+
case t: Literal => tryInstrument(t)
205237
case t: Select => tryInstrument(t)
206238
case _ => InstrumentedParts.notCovered(transform(tree))
207239

@@ -223,10 +255,14 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
223255
inContext(transformCtx(tree)) { // necessary to position inlined code properly
224256
tree match
225257
// simple cases
226-
case tree: (Import | Export | Literal | This | Super | New) => tree
258+
case tree: (Import | Export | This | Super | New) => tree
227259
case tree if tree.isEmpty || tree.isType => tree // empty Thicket, Ident (referring to a type), TypeTree, ...
228260
case tree if !tree.span.exists || tree.span.isZeroExtent => tree // no meaningful position
229261

262+
case tree: Literal =>
263+
val rest = tryInstrument(tree).toTree
264+
rest
265+
230266
// identifier
231267
case tree: Ident =>
232268
tryInstrument(tree).toTree
@@ -280,6 +316,9 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
280316
case tree: CaseDef =>
281317
transformCaseDef(tree)
282318

319+
case tree: ValDef if tree.symbol.is(Inline) =>
320+
tree // transforming inline vals will result in `inline value must be pure` errors
321+
283322
case tree: ValDef =>
284323
// only transform the rhs
285324
val rhs = transform(tree.rhs)
@@ -323,13 +362,13 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
323362
)
324363

325364
case tree: Inlined =>
326-
// Ideally, tree.call would provide precise information about the inlined call,
327-
// and we would use this information for the coverage report.
328-
// But PostTyper simplifies tree.call, so we can't report the actual method that was inlined.
329-
// In any case, the subtrees need to be repositioned right now, otherwise the
330-
// coverage statement will point to a potentially unreachable source file.
331-
val dropped = Inlines.dropInlined(tree) // drop and reposition
332-
transform(dropped) // transform the content of the Inlined
365+
// Inlined code contents might come from another file (or project),
366+
// which means that we cannot clearly designate which part of the inlined code
367+
// was run using the API we are given.
368+
// At best, we can show that the Inlined tree itself was reached.
369+
// Additionally, Scala 2's coverage ignores macro calls entirely,
370+
// so let's do that here too, also for regular inlined calls.
371+
tree
333372

334373
// For everything else just recurse and transform
335374
case _ =>
@@ -559,15 +598,14 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
559598
private def isCompilerIntrinsicMethod(sym: Symbol)(using Context): Boolean =
560599
val owner = sym.maybeOwner
561600
owner.exists && (
562-
owner.eq(defn.AnyClass) ||
563-
owner.isPrimitiveValueClass ||
601+
(owner.eq(defn.AnyClass) && (sym == defn.Any_asInstanceOf || sym == defn.Any_isInstanceOf)) ||
564602
owner.maybeOwner == defn.CompiletimePackageClass
565603
)
566604

567605
object InstrumentCoverage:
568606
val name: String = "instrumentCoverage"
569607
val description: String = "instrument code for coverage checking"
570-
val ExcludeMethodFlags: FlagSet = Synthetic | Artifact | Erased
608+
val ExcludeMethodFlags: FlagSet = Artifact | Erased
571609

572610
/**
573611
* An instrumented Tree, in 3 parts.

tests/coverage/pos/Constructor.scoverage.check

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,23 @@ C
110110
Class
111111
covtest.C
112112
x
113+
161
114+
162
115+
13
116+
<none>
117+
Literal
118+
false
119+
0
120+
false
121+
1
122+
123+
6
124+
Constructor.scala
125+
covtest
126+
C
127+
Class
128+
covtest.C
129+
x
113130
153
114131
158
115132
13
@@ -120,7 +137,7 @@ false
120137
false
121138
def x
122139

123-
6
140+
7
124141
Constructor.scala
125142
covtest
126143
C
@@ -137,7 +154,7 @@ false
137154
false
138155
f(x)
139156

140-
7
157+
8
141158
Constructor.scala
142159
covtest
143160
C
@@ -154,7 +171,24 @@ false
154171
false
155172
x
156173

157-
8
174+
9
175+
Constructor.scala
176+
covtest
177+
C
178+
Class
179+
covtest.C
180+
g
181+
188
182+
189
183+
16
184+
<none>
185+
Literal
186+
false
187+
0
188+
false
189+
2
190+
191+
10
158192
Constructor.scala
159193
covtest
160194
C
@@ -171,7 +205,7 @@ false
171205
false
172206
def g
173207

174-
9
208+
11
175209
Constructor.scala
176210
covtest
177211
O
@@ -188,7 +222,24 @@ false
188222
false
189223
def g
190224

191-
10
225+
12
226+
Constructor.scala
227+
covtest
228+
O
229+
Object
230+
covtest.O
231+
y
232+
231
233+
232
234+
20
235+
<none>
236+
Literal
237+
false
238+
0
239+
false
240+
1
241+
242+
13
192243
Constructor.scala
193244
covtest
194245
O
@@ -205,7 +256,7 @@ false
205256
false
206257
def y
207258

208-
11
259+
14
209260
Constructor.scala
210261
covtest
211262
O
@@ -222,20 +273,3 @@ false
222273
false
223274
g(y)
224275

225-
12
226-
Constructor.scala
227-
covtest
228-
O
229-
Object
230-
covtest.O
231-
<init>
232-
237
233-
238
234-
21
235-
y
236-
Ident
237-
false
238-
0
239-
false
240-
y
241-

tests/coverage/pos/ContextFunctions.scoverage.check

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,23 @@ covtest
4141
Imperative
4242
Class
4343
covtest.Imperative
44+
$anonfun
45+
178
46+
184
47+
9
48+
<none>
49+
Literal
50+
false
51+
0
52+
false
53+
"name"
54+
55+
2
56+
ContextFunctions.scala
57+
covtest
58+
Imperative
59+
Class
60+
covtest.Imperative
4461
readName2
4562
121
4663
134
@@ -52,41 +69,41 @@ false
5269
false
5370
def readName2
5471

55-
2
72+
3
5673
ContextFunctions.scala
5774
covtest
5875
Imperative
5976
Class
6077
covtest.Imperative
6178
readPerson
62-
252
63-
309
64-
14
65-
onError
66-
Apply
79+
243
80+
247
81+
13
82+
<none>
83+
Literal
6784
false
6885
0
6986
false
70-
OnError((e) => readName2(using e)(using s)).onError(None)
87+
null
7188

72-
3
89+
4
7390
ContextFunctions.scala
7491
covtest
7592
Imperative
7693
Class
7794
covtest.Imperative
7895
readPerson
7996
252
80-
295
97+
309
8198
14
82-
<init>
99+
onError
83100
Apply
84101
false
85102
0
86103
false
87-
OnError((e) => readName2(using e)(using s))
104+
OnError((e) => readName2(using e)(using s)).onError(None)
88105

89-
4
106+
5
90107
ContextFunctions.scala
91108
covtest
92109
Imperative
@@ -103,7 +120,7 @@ false
103120
false
104121
readName2(using e)(using s)
105122

106-
5
123+
6
107124
ContextFunctions.scala
108125
covtest
109126
Imperative

0 commit comments

Comments
 (0)