Skip to content

Commit f3fec58

Browse files
authored
Make coverage instrumentation more robust (#16235)
*As with the last coverage PR, don't worry: most of the changes are "expect" files for tests. On top of that, many files have only changed by the order in which the statements are recorded, but this order doesn't matter.* Small changes which, together, make the instrumentation more robust and fix many bugs: 1. Address comments in #15739 by introducing a type `InstrumentedParts`. The initial problem was that `TypeApply` cannot be instrumented in a straightforward way: `expr[T]` cannot be turned into `{invoked(...); expr}[T]` but must be `{invoked(...); expr[T]}`. To do this, we first try to instrument `expr` and then, if it was successfully instrumented, we move the call to `invoked(...)` to the right place. This gives us the following code in `transform`: ```scala case TypeApply(fun, args) => val InstrumentedParts(pre, coverageCall, expr) = tryInstrument(fun) // may generate a call to invoked(...), but it's not part of the resulting tree yet if coverageCall.isEmpty then tree else Block( pre :+ coverageCall, // put the call at the right place (pre contains lifted definitions, if any) cpy.TypeApply(tree)(expr, args) ) ``` 2. Exclude more trees from instrumentation, like `Erased` trees and calls to the parents' constructor in `Template#parents`. 3. Escape special characters in `Serializer`. 4. Reposition `Inlined` trees to the current source in order to avoid referencing an unreachable compilation unit. This might be the most controversial change because I've called `Inlines.dropInlined` 👀. Any suggestion is welcome!
2 parents 3381850 + 02fd9de commit f3fec58

Some content is hidden

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

44 files changed

+2205
-1078
lines changed

compiler/src/dotty/tools/dotc/coverage/Coverage.scala

-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ class Coverage:
1313

1414
/** A statement that can be invoked, and thus counted as "covered" by code coverage tools. */
1515
case class Statement(
16-
source: String,
1716
location: Location,
1817
id: Int,
1918
start: Int,

compiler/src/dotty/tools/dotc/coverage/Location.scala

+10-7
Original file line numberDiff line numberDiff line change
@@ -5,32 +5,35 @@ import ast.tpd._
55
import dotty.tools.dotc.core.Contexts.Context
66
import dotty.tools.dotc.core.Flags.*
77
import java.nio.file.Path
8+
import dotty.tools.dotc.util.SourceFile
89

910
/** Information about the location of a coverable piece of code.
1011
*
1112
* @param packageName name of the enclosing package
1213
* @param className name of the closest enclosing class
1314
* @param fullClassName fully qualified name of the closest enclosing class
1415
* @param classType "type" of the closest enclosing class: Class, Trait or Object
15-
* @param method name of the closest enclosing method
16+
* @param method name of the closest enclosing method
1617
* @param sourcePath absolute path of the source file
1718
*/
1819
final case class Location(
1920
packageName: String,
2021
className: String,
2122
fullClassName: String,
2223
classType: String,
23-
method: String,
24+
methodName: String,
2425
sourcePath: Path
2526
)
2627

2728
object Location:
2829
/** Extracts the location info of a Tree. */
29-
def apply(tree: Tree)(using ctx: Context): Location =
30+
def apply(tree: Tree, source: SourceFile)(using ctx: Context): Location =
3031

31-
val enclosingClass = ctx.owner.denot.enclosingClass
32-
val packageName = ctx.owner.denot.enclosingPackageClass.name.toSimpleName.toString
32+
val ownerDenot = ctx.owner.denot
33+
val enclosingClass = ownerDenot.enclosingClass
34+
val packageName = ownerDenot.enclosingPackageClass.fullName.toSimpleName.toString
3335
val className = enclosingClass.name.toSimpleName.toString
36+
val methodName = ownerDenot.enclosingMethod.name.toSimpleName.toString
3437

3538
val classType: String =
3639
if enclosingClass.is(Trait) then "Trait"
@@ -42,6 +45,6 @@ object Location:
4245
className,
4346
s"$packageName.$className",
4447
classType,
45-
ctx.owner.denot.enclosingMethod.name.toSimpleName.toString(),
46-
ctx.source.file.absolute.jpath
48+
methodName,
49+
source.file.absolute.jpath
4750
)

compiler/src/dotty/tools/dotc/coverage/Serializer.scala

+32-7
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package coverage
44
import java.nio.file.{Path, Paths, Files}
55
import java.io.Writer
66
import scala.language.unsafeNulls
7+
import scala.collection.mutable.StringBuilder
78

89
/**
910
* Serializes scoverage data.
@@ -62,25 +63,49 @@ object Serializer:
6263
def writeStatement(stmt: Statement, writer: Writer): Unit =
6364
// Note: we write 0 for the count because we have not measured the actual coverage at this point
6465
writer.write(s"""${stmt.id}
65-
|${getRelativePath(stmt.location.sourcePath)}
66-
|${stmt.location.packageName}
67-
|${stmt.location.className}
66+
|${getRelativePath(stmt.location.sourcePath).escaped}
67+
|${stmt.location.packageName.escaped}
68+
|${stmt.location.className.escaped}
6869
|${stmt.location.classType}
69-
|${stmt.location.fullClassName}
70-
|${stmt.location.method}
70+
|${stmt.location.fullClassName.escaped}
71+
|${stmt.location.methodName.escaped}
7172
|${stmt.start}
7273
|${stmt.end}
7374
|${stmt.line}
74-
|${stmt.symbolName}
75+
|${stmt.symbolName.escaped}
7576
|${stmt.treeName}
7677
|${stmt.branch}
7778
|0
7879
|${stmt.ignored}
79-
|${stmt.desc}
80+
|${stmt.desc.escaped}
8081
|\f
8182
|""".stripMargin)
8283

8384
writeHeader(writer)
8485
coverage.statements.toSeq
8586
.sortBy(_.id)
8687
.foreach(stmt => writeStatement(stmt, writer))
88+
89+
/** Makes a String suitable for output in the coverage statement data as a single line.
90+
* Escaped characters: '\\' (backslash), '\n', '\r', '\f'
91+
*/
92+
extension (str: String) def escaped: String =
93+
val builder = StringBuilder(str.length)
94+
var i = 0
95+
while
96+
i < str.length
97+
do
98+
str.charAt(i) match
99+
case '\\' =>
100+
builder ++= "\\\\"
101+
case '\n' =>
102+
builder ++= "\\n"
103+
case '\r' =>
104+
builder ++= "\\r"
105+
case '\f' =>
106+
builder ++= "\\f"
107+
case c =>
108+
builder += c
109+
i += 1
110+
end while
111+
builder.result()

0 commit comments

Comments
 (0)