From 7206bfea4118eea34fdcfa37a32a365c45425ad9 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Wed, 22 Jun 2022 17:22:50 +0200 Subject: [PATCH] Avoid instrumentation of inline and erased definitions 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 #15490. --- .../dotc/transform/InstrumentCoverage.scala | 29 ++- .../tools/dotc/coverage/CoverageTests.scala | 1 + tests/coverage/pos/Inlined.scoverage.check | 68 ------ tests/coverage/run/erased-def/test.check | 1 + tests/coverage/run/erased-def/test.scala | 13 ++ .../run/erased-def/test.scoverage.check | 89 ++++++++ tests/coverage/run/inline-def/test.check | 4 + tests/coverage/run/inline-def/test.scala | 18 ++ .../run/inline-def/test.scoverage.check | 208 ++++++++++++++++++ 9 files changed, 351 insertions(+), 80 deletions(-) create mode 100644 tests/coverage/run/erased-def/test.check create mode 100644 tests/coverage/run/erased-def/test.scala create mode 100644 tests/coverage/run/erased-def/test.scoverage.check create mode 100644 tests/coverage/run/inline-def/test.check create mode 100644 tests/coverage/run/inline-def/test.scala create mode 100644 tests/coverage/run/inline-def/test.scoverage.check diff --git a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala index 442bf27d9f6b..7ffc463e1ab3 100644 --- a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala +++ b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala @@ -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)) @@ -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" diff --git a/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala b/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala index 1ec3058415d7..12c2cca34911 100644 --- a/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala +++ b/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala @@ -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") diff --git a/tests/coverage/pos/Inlined.scoverage.check b/tests/coverage/pos/Inlined.scoverage.check index 7c0cb17efece..4be4446e0d91 100644 --- a/tests/coverage/pos/Inlined.scoverage.check +++ b/tests/coverage/pos/Inlined.scoverage.check @@ -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 - -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 - diff --git a/tests/coverage/run/erased-def/test.check b/tests/coverage/run/erased-def/test.check new file mode 100644 index 000000000000..257cc5642cb1 --- /dev/null +++ b/tests/coverage/run/erased-def/test.check @@ -0,0 +1 @@ +foo diff --git a/tests/coverage/run/erased-def/test.scala b/tests/coverage/run/erased-def/test.scala new file mode 100644 index 000000000000..2d8f576f08d2 --- /dev/null +++ b/tests/coverage/run/erased-def/test.scala @@ -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") diff --git a/tests/coverage/run/erased-def/test.scoverage.check b/tests/coverage/run/erased-def/test.scoverage.check new file mode 100644 index 000000000000..d42012c8da8d --- /dev/null +++ b/tests/coverage/run/erased-def/test.scoverage.check @@ -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 + +A +Class +.A +foo +103 +110 +4 +foo +DefDef +false +0 +false +def foo + +1 +erased-def/test.scala + +test$package$ +Object +.test$package$ +Test +179 +182 +8 + +Apply +false +0 +false +A() + +2 +erased-def/test.scala + +test$package$ +Object +.test$package$ +Test +289 +303 +12 +println +Apply +false +0 +false +println("foo") + +3 +erased-def/test.scala + +test$package$ +Object +.test$package$ +Test +146 +160 +7 +Test +DefDef +false +0 +false +@main +def Test + diff --git a/tests/coverage/run/inline-def/test.check b/tests/coverage/run/inline-def/test.check new file mode 100644 index 000000000000..2d4b49ca1fdf --- /dev/null +++ b/tests/coverage/run/inline-def/test.check @@ -0,0 +1,4 @@ +1 +foo +bar +foo diff --git a/tests/coverage/run/inline-def/test.scala b/tests/coverage/run/inline-def/test.scala new file mode 100644 index 000000000000..967eb23068c7 --- /dev/null +++ b/tests/coverage/run/inline-def/test.scala @@ -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) diff --git a/tests/coverage/run/inline-def/test.scoverage.check b/tests/coverage/run/inline-def/test.scoverage.check new file mode 100644 index 000000000000..42ecf6d9b9a4 --- /dev/null +++ b/tests/coverage/run/inline-def/test.scoverage.check @@ -0,0 +1,208 @@ +# 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 +inline-def/test.scala + +A +Class +.A + +66 +67 +4 + +Apply +false +0 +false +B + +1 +inline-def/test.scala + +A +Class +.A +foo$retainedBody +134 +148 +7 +toString +Apply +false +0 +false +"foo".toString + +2 +inline-def/test.scala + +A +Class +.A +foo$retainedBody +134 +134 +7 +foo$retainedBody +DefDef +false +0 +false + + +3 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +225 +228 +12 + +Apply +false +0 +false +A() + +4 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +231 +243 +13 +println +Apply +false +0 +false +println(a.x) + +5 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +134 +148 +7 +toString +Apply +false +0 +false +"foo".toString + +6 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +246 +260 +14 +println +Apply +false +0 +false +println(a.foo) + +7 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +176 +190 +8 +toString +Apply +false +0 +false +"bar".toString + +8 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +263 +277 +15 +println +Apply +false +0 +false +println(a.bar) + +9 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +295 +309 +17 +println +Apply +false +0 +false +println(b.foo) + +10 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +192 +206 +11 +Test +DefDef +false +0 +false +@main +def Test +