Skip to content

Commit

Permalink
Avoid instrumentation of inline and erased definitions
Browse files Browse the repository at this point in the history
These methods will not generate code and therefore do not need to be
instrumented. Note that a retained inline method will have a `$retained`
variant that will be instrumented.

Found this issue while looking into scala#15490.
  • Loading branch information
nicolasstucki committed Jun 22, 2022
1 parent 4e5e055 commit 7206bfe
Show file tree
Hide file tree
Showing 9 changed files with 351 additions and 80 deletions.
29 changes: 17 additions & 12 deletions compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala
Original file line number Diff line number Diff line change
Expand Up @@ -131,17 +131,22 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
cpy.ValDef(tree)(rhs = rhs)

case tree: DefDef =>
// Only transform the params (for the default values) and the rhs.
val paramss = transformParamss(tree.paramss)
val rhs = transform(tree.rhs)
val finalRhs =
if canInstrumentDefDef(tree) then
// Ensure that the rhs is always instrumented, if possible
instrumentBody(tree, rhs)
else
rhs
cpy.DefDef(tree)(tree.name, paramss, tree.tpt, finalRhs)

if tree.symbol.isOneOf(Inline | Erased) then
// Inline and erased definitions will not be in the generated code and therefore do not need to be instrumented.
// Note that a retained inline method will have a `$retained` variant that will be instrumented.
tree
else
// Only transform the params (for the default values) and the rhs.
val paramss = transformParamss(tree.paramss)
val rhs = transform(tree.rhs)
val finalRhs =
if canInstrumentDefDef(tree) then
// Ensure that the rhs is always instrumented, if possible
instrumentBody(tree, rhs)
else
rhs
cpy.DefDef(tree)(tree.name, paramss, tree.tpt, finalRhs)
end if
case tree: PackageDef =>
// only transform the statements of the package
cpy.PackageDef(tree)(tree.pid, transform(tree.stats))
Expand Down Expand Up @@ -330,4 +335,4 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:

object InstrumentCoverage:
val name: String = "instrumentCoverage"
val description: String = "instrument code for coverage cheking"
val description: String = "instrument code for coverage checking"
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class CoverageTests:
val expected = fixWindowsPaths(Files.readAllLines(expectFile).asScala)
val obtained = fixWindowsPaths(Files.readAllLines(targetFile).asScala)
if expected != obtained then
// FIXME: zip will drop part of the output if one is shorter (i.e. will not print anything of one is a refix of the other)
for ((exp, actual),i) <- expected.zip(obtained).filter(_ != _).zipWithIndex do
Console.err.println(s"wrong line ${i+1}:")
Console.err.println(s" expected: $exp")
Expand Down
68 changes: 0 additions & 68 deletions tests/coverage/pos/Inlined.scoverage.check
Original file line number Diff line number Diff line change
Expand Up @@ -273,71 +273,3 @@ false
false
def testInlined

15
Inlined.scala
covtest
Inlined$package$
Object
covtest.Inlined$package$
assert
288
315
10
Scala3RunTime
Select
false
0
false
scala.runtime.Scala3RunTime

16
Inlined.scala
covtest
Inlined$package$
Object
covtest.Inlined$package$
assert
288
330
10
assertFailed
Apply
false
0
false
scala.runtime.Scala3RunTime.assertFailed()

17
Inlined.scala
covtest
Inlined$package$
Object
covtest.Inlined$package$
assert
288
330
10
<none>
Block
true
0
false
scala.runtime.Scala3RunTime.assertFailed()

18
Inlined.scala
covtest
Inlined$package$
Object
covtest.Inlined$package$
assert
202
231
9
assert
DefDef
false
0
false
transparent inline def assert

1 change: 1 addition & 0 deletions tests/coverage/run/erased-def/test.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
foo
13 changes: 13 additions & 0 deletions tests/coverage/run/erased-def/test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import scala.language.experimental.erasedDefinitions

class A:
erased def x: String = "x".toString
def foo(erased s: String): String = "foo"

@main
def Test: Unit =
val a = A()
// FIXME: coverage should not track erased arguments and statements
// a.x
// println(a.foo(a.x))
println("foo")
89 changes: 89 additions & 0 deletions tests/coverage/run/erased-def/test.scoverage.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Coverage data, format version: 3.0
# Statement data:
# - id
# - source path
# - package name
# - class name
# - class type (Class, Object or Trait)
# - full class name
# - method name
# - start offset
# - end offset
# - line number
# - symbol name
# - tree name
# - is branch
# - invocations count
# - is ignored
# - description (can be multi-line)
# ' ' sign
# ------------------------------------------
0
erased-def/test.scala
<empty>
A
Class
<empty>.A
foo
103
110
4
foo
DefDef
false
0
false
def foo

1
erased-def/test.scala
<empty>
test$package$
Object
<empty>.test$package$
Test
179
182
8
<init>
Apply
false
0
false
A()

2
erased-def/test.scala
<empty>
test$package$
Object
<empty>.test$package$
Test
289
303
12
println
Apply
false
0
false
println("foo")

3
erased-def/test.scala
<empty>
test$package$
Object
<empty>.test$package$
Test
146
160
7
Test
DefDef
false
0
false
@main
def Test

4 changes: 4 additions & 0 deletions tests/coverage/run/inline-def/test.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
1
foo
bar
foo
18 changes: 18 additions & 0 deletions tests/coverage/run/inline-def/test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
abstract class B:
val x: Int
def foo: String

class A extends B:
inline val x = 1
inline val y = 2
inline def foo: String = "foo".toString
inline def bar: String = "bar".toString

@main
def Test: Unit =
val a = A()
println(a.x)
println(a.foo)
println(a.bar)
val b: B = a
println(b.foo)
Loading

0 comments on commit 7206bfe

Please sign in to comment.