From 817d72f2835441fac85f63a0c0815cf5a94063a7 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Fri, 3 Jan 2025 12:14:32 +0100 Subject: [PATCH 1/4] Fix macro suspensions breaking coverage --- compiler/src/dotty/tools/dotc/core/Contexts.scala | 5 +++++ .../src/dotty/tools/dotc/coverage/Serializer.scala | 14 ++++++++------ .../tools/dotc/transform/InstrumentCoverage.scala | 9 +++------ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 7f5779bb6127..4d2f16285553 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -982,6 +982,11 @@ object Contexts { /** Was best effort file used during compilation? */ private[core] var usedBestEffortTasty = false + /** Counter to assign a unique id to each statement (for coverage) */ + private[dotc] var coverageStatementId = 0 + /** A variable denoting if the coverage file already written to (for coverage) */ + private[dotc] var coverageStartedWriting = false + // Types state /** A table for hash consing unique types */ private[core] val uniques: Uniques = Uniques() diff --git a/compiler/src/dotty/tools/dotc/coverage/Serializer.scala b/compiler/src/dotty/tools/dotc/coverage/Serializer.scala index 26efa8934e00..5f5b7369a772 100644 --- a/compiler/src/dotty/tools/dotc/coverage/Serializer.scala +++ b/compiler/src/dotty/tools/dotc/coverage/Serializer.scala @@ -1,10 +1,11 @@ package dotty.tools.dotc package coverage -import java.nio.file.{Path, Paths, Files} +import java.nio.file.{Path, Paths, Files, StandardOpenOption} import java.io.Writer import scala.language.unsafeNulls import scala.collection.mutable.StringBuilder +import dotty.tools.dotc.core.Contexts.Context /** * Serializes scoverage data. @@ -16,12 +17,12 @@ object Serializer: private val CoverageDataFormatVersion = "3.0" /** Write out coverage data to the given data directory, using the default coverage filename */ - def serialize(coverage: Coverage, dataDir: String, sourceRoot: String): Unit = + def serialize(coverage: Coverage, dataDir: String, sourceRoot: String)(using Context): Unit = serialize(coverage, Paths.get(dataDir, CoverageFileName).toAbsolutePath, Paths.get(sourceRoot).toAbsolutePath) /** Write out coverage data to a file. */ - def serialize(coverage: Coverage, file: Path, sourceRoot: Path): Unit = - val writer = Files.newBufferedWriter(file) + def serialize(coverage: Coverage, file: Path, sourceRoot: Path)(using Context): Unit = + val writer = Files.newBufferedWriter(file, StandardOpenOption.CREATE, StandardOpenOption.APPEND) try serialize(coverage, writer, sourceRoot) finally @@ -29,7 +30,7 @@ object Serializer: /** Write out coverage data (info about each statement that can be covered) to a writer. */ - def serialize(coverage: Coverage, writer: Writer, sourceRoot: Path): Unit = + def serialize(coverage: Coverage, writer: Writer, sourceRoot: Path)(using ctx: Context): Unit = def getRelativePath(filePath: Path): String = // We need to normalize the path here because the relativizing paths containing '.' or '..' differs between Java versions @@ -81,7 +82,8 @@ object Serializer: |\f |""".stripMargin) - writeHeader(writer) + if (!ctx.base.coverageStartedWriting) writeHeader(writer) + ctx.base.coverageStartedWriting = true coverage.statements.toSeq .sortBy(_.id) .foreach(stmt => writeStatement(stmt, writer)) diff --git a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala index f5e0f8c63b58..1f7f88e66fe4 100644 --- a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala +++ b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala @@ -38,9 +38,6 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: override def isEnabled(using ctx: Context) = ctx.settings.coverageOutputDir.value.nonEmpty - // counter to assign a unique id to each statement - private var statementId = 0 - // stores all instrumented statements private val coverage = Coverage() @@ -54,7 +51,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: val dataDir = File(outputPath) val newlyCreated = dataDir.mkdirs() - if !newlyCreated then + if !newlyCreated && !ctx.base.coverageStartedWriting then // If the directory existed before, let's clean it up. dataDir.listFiles.nn .filter(_.nn.getName.nn.startsWith("scoverage")) @@ -110,8 +107,8 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: * @return the statement's id */ private def recordStatement(tree: Tree, pos: SourcePosition, branch: Boolean)(using ctx: Context): Int = - val id = statementId - statementId += 1 + val id = ctx.base.coverageStatementId + ctx.base.coverageStatementId += 1 val sourceFile = pos.source val statement = Statement( From 66ad0cd1dc66e4388de617aa790e309179159a34 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Fri, 3 Jan 2025 12:17:27 +0100 Subject: [PATCH 2/4] Support compiling multiple files in coverage tests --- .../tools/dotc/coverage/CoverageTests.scala | 41 +++-- .../dotty/tools/vulpix/ParallelTesting.scala | 24 ++- tests/coverage/run/i16940/i16940.check | 1 + ...0.scoverage.check => test.scoverage.check} | 0 .../coverage/run/i18233-min/i182233-min.check | 2 + ...n.scoverage.check => test.scoverage.check} | 0 tests/coverage/run/i18233/i18233.check | 1 + ...3.scoverage.check => test.scoverage.check} | 0 .../{test.check => JavaObject.check} | 0 tests/coverage/run/macro-suspend/Macro.check | 1 + tests/coverage/run/macro-suspend/Macro.scala | 8 + tests/coverage/run/macro-suspend/Test.scala | 4 + .../run/macro-suspend/test.scoverage.check | 173 ++++++++++++++++++ .../coverage/run/varargs/JavaVarargs_1.check | 3 + .../run/varargs/{test_1.check => test.check} | 0 ...1.scoverage.check => test.scoverage.check} | 0 16 files changed, 236 insertions(+), 22 deletions(-) create mode 100644 tests/coverage/run/i16940/i16940.check rename tests/coverage/run/i16940/{i16940.scoverage.check => test.scoverage.check} (100%) create mode 100644 tests/coverage/run/i18233-min/i182233-min.check rename tests/coverage/run/i18233-min/{i18233-min.scoverage.check => test.scoverage.check} (100%) create mode 100644 tests/coverage/run/i18233/i18233.check rename tests/coverage/run/i18233/{i18233.scoverage.check => test.scoverage.check} (100%) rename tests/coverage/run/java-methods/{test.check => JavaObject.check} (100%) create mode 100644 tests/coverage/run/macro-suspend/Macro.check create mode 100644 tests/coverage/run/macro-suspend/Macro.scala create mode 100644 tests/coverage/run/macro-suspend/Test.scala create mode 100644 tests/coverage/run/macro-suspend/test.scoverage.check create mode 100644 tests/coverage/run/varargs/JavaVarargs_1.check rename tests/coverage/run/varargs/{test_1.check => test.check} (100%) rename tests/coverage/run/varargs/{test_1.scoverage.check => test.scoverage.check} (100%) diff --git a/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala b/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala index 7efa1f6f564e..d16b1c1d946d 100644 --- a/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala +++ b/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala @@ -54,15 +54,27 @@ class CoverageTests: lines end fixWindowsPaths - def runOnFile(p: Path): Boolean = - scalaFile.matches(p) && - (Properties.testsFilter.isEmpty || Properties.testsFilter.exists(p.toString.contains)) + def runOnFileOrDir(p: Path): Boolean = + (scalaFile.matches(p) || Files.isDirectory(p)) + && (p != dir) + && (Properties.testsFilter.isEmpty || Properties.testsFilter.exists(p.toString.contains)) + + Files.walk(dir, 1).filter(runOnFileOrDir).forEach(path => { + // measurement files only exist in the "run" category + // as these are generated at runtime by the scala.runtime.coverage.Invoker + val (targetDir, expectFile, expectMeasurementFile) = + if Files.isDirectory(path) then + val dirName = path.getFileName().toString + assert(!Files.walk(path).filter(scalaFile.matches(_)).toList.isEmpty, s"No scala files found in test directory: ${path}") + val targetDir = computeCoverageInTmp(path, isDirectory = true, dir, run) + (targetDir, path.resolve(s"test.scoverage.check"), path.resolve(s"test.measurement.check")) + else + val fileName = path.getFileName.toString.stripSuffix(".scala") + val targetDir = computeCoverageInTmp(path, isDirectory = false, dir, run) + (targetDir, path.resolveSibling(s"${fileName}.scoverage.check"), path.resolveSibling(s"${fileName}.measurement.check")) - Files.walk(dir).filter(runOnFile).forEach(path => { - val fileName = path.getFileName.toString.stripSuffix(".scala") - val targetDir = computeCoverageInTmp(path, dir, run) val targetFile = targetDir.resolve(s"scoverage.coverage") - val expectFile = path.resolveSibling(s"$fileName.scoverage.check") + if updateCheckFiles then Files.copy(targetFile, expectFile, StandardCopyOption.REPLACE_EXISTING) else @@ -72,9 +84,6 @@ class CoverageTests: val instructions = FileDiff.diffMessage(expectFile.toString, targetFile.toString) fail(s"Coverage report differs from expected data.\n$instructions") - // measurement files only exist in the "run" category - // as these are generated at runtime by the scala.runtime.coverage.Invoker - val expectMeasurementFile = path.resolveSibling(s"$fileName.measurement.check") if run && Files.exists(expectMeasurementFile) then // Note that this assumes that the test invoked was single threaded, @@ -95,14 +104,20 @@ class CoverageTests: }) /** Generates the coverage report for the given input file, in a temporary directory. */ - def computeCoverageInTmp(inputFile: Path, sourceRoot: Path, run: Boolean)(using TestGroup): Path = + def computeCoverageInTmp(inputFile: Path, isDirectory: Boolean, sourceRoot: Path, run: Boolean)(using TestGroup): Path = val target = Files.createTempDirectory("coverage") val options = defaultOptions.and("-Ycheck:instrumentCoverage", "-coverage-out", target.toString, "-sourceroot", sourceRoot.toString) if run then - val test = compileDir(inputFile.getParent.toString, options) + val path = if isDirectory then inputFile.toString else inputFile.getParent.toString + val test = compileDir(path, options) + test.checkFilePaths.foreach { checkFilePath => + assert(checkFilePath.exists, s"Expected checkfile for $path $checkFilePath does not exist.") + } test.checkRuns() else - val test = compileFile(inputFile.toString, options) + val test = + if isDirectory then compileDir(inputFile.toString, options) + else compileFile(inputFile.toString, options) test.checkCompile() target diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index e7e5936a4b29..29e64163b833 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -258,15 +258,7 @@ trait ParallelTesting extends RunnerOrchestration { self => * For a given test source, returns a check file against which the result of the test run * should be compared. Is used by implementations of this trait. */ - final def checkFile(testSource: TestSource): Option[JFile] = (testSource match { - case ts: JointCompilationSource => - ts.files.collectFirst { - case f if !f.isDirectory => - new JFile(f.getPath.replaceFirst("\\.(scala|java)$", ".check")) - } - case ts: SeparateCompilationSource => - Option(new JFile(ts.dir.getPath + ".check")) - }).filter(_.exists) + final def checkFile(testSource: TestSource): Option[JFile] = (CompilationLogic.checkFilePath(testSource)).filter(_.exists) /** * Checks if the given actual lines are the same as the ones in the check file. @@ -343,6 +335,18 @@ trait ParallelTesting extends RunnerOrchestration { self => } } + object CompilationLogic { + private[ParallelTesting] def checkFilePath(testSource: TestSource) = testSource match { + case ts: JointCompilationSource => + ts.files.collectFirst { + case f if !f.isDirectory => + new JFile(f.getPath.replaceFirst("\\.(scala|java)$", ".check")) + } + case ts: SeparateCompilationSource => + Option(new JFile(ts.dir.getPath + ".check")) + } + } + /** Each `Test` takes the `testSources` and performs the compilation and assertions * according to the implementing class "neg", "run" or "pos". */ @@ -1157,6 +1161,8 @@ trait ParallelTesting extends RunnerOrchestration { self => def this(targets: List[TestSource]) = this(targets, 1, true, None, false, false) + def checkFilePaths: List[JFile] = targets.map(CompilationLogic.checkFilePath).flatten + def copy(targets: List[TestSource], times: Int = times, shouldDelete: Boolean = shouldDelete, diff --git a/tests/coverage/run/i16940/i16940.check b/tests/coverage/run/i16940/i16940.check new file mode 100644 index 000000000000..0cfbf08886fc --- /dev/null +++ b/tests/coverage/run/i16940/i16940.check @@ -0,0 +1 @@ +2 diff --git a/tests/coverage/run/i16940/i16940.scoverage.check b/tests/coverage/run/i16940/test.scoverage.check similarity index 100% rename from tests/coverage/run/i16940/i16940.scoverage.check rename to tests/coverage/run/i16940/test.scoverage.check diff --git a/tests/coverage/run/i18233-min/i182233-min.check b/tests/coverage/run/i18233-min/i182233-min.check new file mode 100644 index 000000000000..a4bb477e9e1f --- /dev/null +++ b/tests/coverage/run/i18233-min/i182233-min.check @@ -0,0 +1,2 @@ +List() +List(abc, def) diff --git a/tests/coverage/run/i18233-min/i18233-min.scoverage.check b/tests/coverage/run/i18233-min/test.scoverage.check similarity index 100% rename from tests/coverage/run/i18233-min/i18233-min.scoverage.check rename to tests/coverage/run/i18233-min/test.scoverage.check diff --git a/tests/coverage/run/i18233/i18233.check b/tests/coverage/run/i18233/i18233.check new file mode 100644 index 000000000000..a68818270123 --- /dev/null +++ b/tests/coverage/run/i18233/i18233.check @@ -0,0 +1 @@ +Baz diff --git a/tests/coverage/run/i18233/i18233.scoverage.check b/tests/coverage/run/i18233/test.scoverage.check similarity index 100% rename from tests/coverage/run/i18233/i18233.scoverage.check rename to tests/coverage/run/i18233/test.scoverage.check diff --git a/tests/coverage/run/java-methods/test.check b/tests/coverage/run/java-methods/JavaObject.check similarity index 100% rename from tests/coverage/run/java-methods/test.check rename to tests/coverage/run/java-methods/JavaObject.check diff --git a/tests/coverage/run/macro-suspend/Macro.check b/tests/coverage/run/macro-suspend/Macro.check new file mode 100644 index 000000000000..80a62d1af279 --- /dev/null +++ b/tests/coverage/run/macro-suspend/Macro.check @@ -0,0 +1 @@ +>>> hello <<< diff --git a/tests/coverage/run/macro-suspend/Macro.scala b/tests/coverage/run/macro-suspend/Macro.scala new file mode 100644 index 000000000000..dffe91a9a9b1 --- /dev/null +++ b/tests/coverage/run/macro-suspend/Macro.scala @@ -0,0 +1,8 @@ +import scala.quoted.{Expr, Quotes} + +object Macro: + inline def decorate(inline s: String): String = ${ decorateQuotes('s) } + def decorateQuotes(s: Expr[String])(using Quotes): Expr[String] = '{ ">>> " + $s + " <<<" } + +object Greeting: + def greet() = "hello" diff --git a/tests/coverage/run/macro-suspend/Test.scala b/tests/coverage/run/macro-suspend/Test.scala new file mode 100644 index 000000000000..09cf1c36526c --- /dev/null +++ b/tests/coverage/run/macro-suspend/Test.scala @@ -0,0 +1,4 @@ +object Test: + def main(args: Array[String]): Unit = + println(Macro.decorate(Greeting.greet())) + diff --git a/tests/coverage/run/macro-suspend/test.scoverage.check b/tests/coverage/run/macro-suspend/test.scoverage.check new file mode 100644 index 000000000000..759897eb7747 --- /dev/null +++ b/tests/coverage/run/macro-suspend/test.scoverage.check @@ -0,0 +1,173 @@ +# 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 +macro-suspend/Macro.scala + +Macro +Object +.Macro +decorateQuotes +195 +215 +5 +unpickleExprV2 +Apply +false +0 +false +">>> " + $s + " <<<" + +1 +macro-suspend/Macro.scala + +Macro +Object +.Macro +$anonfun +205 +206 +5 +apply +Apply +false +0 +false +s + +2 +macro-suspend/Macro.scala + +Macro +Object +.Macro +decorateQuotes +126 +144 +5 +decorateQuotes +DefDef +false +0 +false +def decorateQuotes + +3 +macro-suspend/Macro.scala + +Greeting +Object +.Greeting +greet +238 +247 +8 +greet +DefDef +false +0 +false +def greet + +4 +macro-suspend/Test.scala + +Test +Object +.Test +main +57 +98 +3 +println +Apply +false +0 +false +println(Macro.decorate(Greeting.greet())) + +5 +macro-suspend/Test.scala + +Test +Object +.Test +main +65 +97 +3 ++ +Apply +false +0 +false +Macro.decorate(Greeting.greet()) + +6 +macro-suspend/Test.scala + +Test +Object +.Test +main +65 +97 +3 ++ +Apply +false +0 +false +Macro.decorate(Greeting.greet()) + +7 +macro-suspend/Test.scala + +Test +Object +.Test +main +80 +96 +3 +greet +Apply +false +0 +false +Greeting.greet() + +8 +macro-suspend/Test.scala + +Test +Object +.Test +main +15 +23 +2 +main +DefDef +false +0 +false +def main + diff --git a/tests/coverage/run/varargs/JavaVarargs_1.check b/tests/coverage/run/varargs/JavaVarargs_1.check new file mode 100644 index 000000000000..24f879b660ff --- /dev/null +++ b/tests/coverage/run/varargs/JavaVarargs_1.check @@ -0,0 +1,3 @@ +first0 +first0 +first3 diff --git a/tests/coverage/run/varargs/test_1.check b/tests/coverage/run/varargs/test.check similarity index 100% rename from tests/coverage/run/varargs/test_1.check rename to tests/coverage/run/varargs/test.check diff --git a/tests/coverage/run/varargs/test_1.scoverage.check b/tests/coverage/run/varargs/test.scoverage.check similarity index 100% rename from tests/coverage/run/varargs/test_1.scoverage.check rename to tests/coverage/run/varargs/test.scoverage.check From d07040d28a7a7f2fbd12bd9a40cbdcde98e6f6ed Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Fri, 3 Jan 2025 13:44:39 +0100 Subject: [PATCH 3/4] Fix coverage for macro suspension after InstrumentCoverage --- .../src/dotty/tools/dotc/core/Contexts.scala | 10 ++- .../dotty/tools/dotc/coverage/Coverage.scala | 12 +++ .../tools/dotc/coverage/Serializer.scala | 14 ++- .../dotc/transform/InstrumentCoverage.scala | 13 ++- .../tools/dotc/coverage/CoverageTests.scala | 2 +- .../pos/macro-late-suspend/Test.scala | 13 +++ .../pos/macro-late-suspend/UsesTest.scala | 3 + .../macro-late-suspend/VisitorMacros.scala | 12 +++ .../macro-late-suspend/test.scoverage.check | 88 +++++++++++++++++++ 9 files changed, 146 insertions(+), 21 deletions(-) create mode 100644 tests/coverage/pos/macro-late-suspend/Test.scala create mode 100644 tests/coverage/pos/macro-late-suspend/UsesTest.scala create mode 100644 tests/coverage/pos/macro-late-suspend/VisitorMacros.scala create mode 100644 tests/coverage/pos/macro-late-suspend/test.scoverage.check diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 4d2f16285553..d6d726c8e92d 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -41,6 +41,7 @@ import util.Store import plugins.* import java.util.concurrent.atomic.AtomicInteger import java.nio.file.InvalidPathException +import dotty.tools.dotc.coverage.Coverage object Contexts { @@ -982,10 +983,11 @@ object Contexts { /** Was best effort file used during compilation? */ private[core] var usedBestEffortTasty = false - /** Counter to assign a unique id to each statement (for coverage) */ - private[dotc] var coverageStatementId = 0 - /** A variable denoting if the coverage file already written to (for coverage) */ - private[dotc] var coverageStartedWriting = false + /** Stores all instrumented statements (for InstrumentCoverage). + * We need this information to be persisted across different runs, + * so it's stored here. + */ + private[dotc] var coverage: Coverage = Coverage() // Types state /** A table for hash consing unique types */ diff --git a/compiler/src/dotty/tools/dotc/coverage/Coverage.scala b/compiler/src/dotty/tools/dotc/coverage/Coverage.scala index 98e67178fb69..3061bfa4ee5c 100644 --- a/compiler/src/dotty/tools/dotc/coverage/Coverage.scala +++ b/compiler/src/dotty/tools/dotc/coverage/Coverage.scala @@ -2,15 +2,27 @@ package dotty.tools.dotc package coverage import scala.collection.mutable +import java.nio.file.Path /** Holds a list of statements to include in the coverage reports. */ class Coverage: private val statementsById = new mutable.LongMap[Statement](256) + private var statementId: Int = 0 + + def nextStatementId(): Int = + statementId += 1 + statementId - 1 + + def statements: Iterable[Statement] = statementsById.values def addStatement(stmt: Statement): Unit = statementsById(stmt.id) = stmt + def removeStatementsFromFile(sourcePath: Path) = + val removedIds = statements.filter(_.location.sourcePath == sourcePath).map(_.id.toLong) + removedIds.foreach(statementsById.remove) + /** * A statement that can be invoked, and thus counted as "covered" by code coverage tools. diff --git a/compiler/src/dotty/tools/dotc/coverage/Serializer.scala b/compiler/src/dotty/tools/dotc/coverage/Serializer.scala index 5f5b7369a772..26efa8934e00 100644 --- a/compiler/src/dotty/tools/dotc/coverage/Serializer.scala +++ b/compiler/src/dotty/tools/dotc/coverage/Serializer.scala @@ -1,11 +1,10 @@ package dotty.tools.dotc package coverage -import java.nio.file.{Path, Paths, Files, StandardOpenOption} +import java.nio.file.{Path, Paths, Files} import java.io.Writer import scala.language.unsafeNulls import scala.collection.mutable.StringBuilder -import dotty.tools.dotc.core.Contexts.Context /** * Serializes scoverage data. @@ -17,12 +16,12 @@ object Serializer: private val CoverageDataFormatVersion = "3.0" /** Write out coverage data to the given data directory, using the default coverage filename */ - def serialize(coverage: Coverage, dataDir: String, sourceRoot: String)(using Context): Unit = + def serialize(coverage: Coverage, dataDir: String, sourceRoot: String): Unit = serialize(coverage, Paths.get(dataDir, CoverageFileName).toAbsolutePath, Paths.get(sourceRoot).toAbsolutePath) /** Write out coverage data to a file. */ - def serialize(coverage: Coverage, file: Path, sourceRoot: Path)(using Context): Unit = - val writer = Files.newBufferedWriter(file, StandardOpenOption.CREATE, StandardOpenOption.APPEND) + def serialize(coverage: Coverage, file: Path, sourceRoot: Path): Unit = + val writer = Files.newBufferedWriter(file) try serialize(coverage, writer, sourceRoot) finally @@ -30,7 +29,7 @@ object Serializer: /** Write out coverage data (info about each statement that can be covered) to a writer. */ - def serialize(coverage: Coverage, writer: Writer, sourceRoot: Path)(using ctx: Context): Unit = + def serialize(coverage: Coverage, writer: Writer, sourceRoot: Path): Unit = def getRelativePath(filePath: Path): String = // We need to normalize the path here because the relativizing paths containing '.' or '..' differs between Java versions @@ -82,8 +81,7 @@ object Serializer: |\f |""".stripMargin) - if (!ctx.base.coverageStartedWriting) writeHeader(writer) - ctx.base.coverageStartedWriting = true + writeHeader(writer) coverage.statements.toSeq .sortBy(_.id) .foreach(stmt => writeStatement(stmt, writer)) diff --git a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala index 1f7f88e66fe4..8a8cbf037dc0 100644 --- a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala +++ b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala @@ -38,9 +38,6 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: override def isEnabled(using ctx: Context) = ctx.settings.coverageOutputDir.value.nonEmpty - // stores all instrumented statements - private val coverage = Coverage() - private var coverageExcludeClasslikePatterns: List[Pattern] = Nil private var coverageExcludeFilePatterns: List[Pattern] = Nil @@ -51,7 +48,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: val dataDir = File(outputPath) val newlyCreated = dataDir.mkdirs() - if !newlyCreated && !ctx.base.coverageStartedWriting then + if !newlyCreated then // If the directory existed before, let's clean it up. dataDir.listFiles.nn .filter(_.nn.getName.nn.startsWith("scoverage")) @@ -61,9 +58,10 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: coverageExcludeClasslikePatterns = ctx.settings.coverageExcludeClasslikes.value.map(_.r.pattern) coverageExcludeFilePatterns = ctx.settings.coverageExcludeFiles.value.map(_.r.pattern) + ctx.base.coverage.removeStatementsFromFile(ctx.compilationUnit.source.file.absolute.jpath) super.run - Serializer.serialize(coverage, outputPath, ctx.settings.sourceroot.value) + Serializer.serialize(ctx.base.coverage, outputPath, ctx.settings.sourceroot.value) private def isClassIncluded(sym: Symbol)(using Context): Boolean = val fqn = sym.fullName.toText(ctx.printerFn(ctx)).show @@ -107,8 +105,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: * @return the statement's id */ private def recordStatement(tree: Tree, pos: SourcePosition, branch: Boolean)(using ctx: Context): Int = - val id = ctx.base.coverageStatementId - ctx.base.coverageStatementId += 1 + val id = ctx.base.coverage.nextStatementId() val sourceFile = pos.source val statement = Statement( @@ -124,7 +121,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: treeName = tree.getClass.getSimpleName.nn, branch ) - coverage.addStatement(statement) + ctx.base.coverage.addStatement(statement) id /** diff --git a/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala b/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala index d16b1c1d946d..f6460180cab9 100644 --- a/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala +++ b/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala @@ -65,7 +65,7 @@ class CoverageTests: val (targetDir, expectFile, expectMeasurementFile) = if Files.isDirectory(path) then val dirName = path.getFileName().toString - assert(!Files.walk(path).filter(scalaFile.matches(_)).toList.isEmpty, s"No scala files found in test directory: ${path}") + assert(!Files.walk(path).filter(scalaFile.matches(_)).toArray.isEmpty, s"No scala files found in test directory: ${path}") val targetDir = computeCoverageInTmp(path, isDirectory = true, dir, run) (targetDir, path.resolve(s"test.scoverage.check"), path.resolve(s"test.measurement.check")) else diff --git a/tests/coverage/pos/macro-late-suspend/Test.scala b/tests/coverage/pos/macro-late-suspend/Test.scala new file mode 100644 index 000000000000..bf008583c7d9 --- /dev/null +++ b/tests/coverage/pos/macro-late-suspend/Test.scala @@ -0,0 +1,13 @@ +package example + +sealed trait Test + +object Test { + case object Foo extends Test + + val visitorType = mkVisitorType[Test] + + trait Visitor[A] { + type V[a] = visitorType.Out[a] + } +} diff --git a/tests/coverage/pos/macro-late-suspend/UsesTest.scala b/tests/coverage/pos/macro-late-suspend/UsesTest.scala new file mode 100644 index 000000000000..803e93c328c9 --- /dev/null +++ b/tests/coverage/pos/macro-late-suspend/UsesTest.scala @@ -0,0 +1,3 @@ +package example + +val _ = Test.Foo diff --git a/tests/coverage/pos/macro-late-suspend/VisitorMacros.scala b/tests/coverage/pos/macro-late-suspend/VisitorMacros.scala new file mode 100644 index 000000000000..49ada0ff2180 --- /dev/null +++ b/tests/coverage/pos/macro-late-suspend/VisitorMacros.scala @@ -0,0 +1,12 @@ +package example + +import scala.quoted.* + +private def mkVisitorTypeImpl[T: Type](using q: Quotes): Expr[VisitorType[T]] = + '{new VisitorType[T]{}} + +transparent inline def mkVisitorType[T]: VisitorType[T] = ${ mkVisitorTypeImpl[T] } + +trait VisitorType[T] { + type Out[A] +} diff --git a/tests/coverage/pos/macro-late-suspend/test.scoverage.check b/tests/coverage/pos/macro-late-suspend/test.scoverage.check new file mode 100644 index 000000000000..f962705ed2ce --- /dev/null +++ b/tests/coverage/pos/macro-late-suspend/test.scoverage.check @@ -0,0 +1,88 @@ +# 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 +# ------------------------------------------ +1 +macro-late-suspend/VisitorMacros.scala +example +VisitorMacros$package +Object +example.VisitorMacros$package +mkVisitorTypeImpl +124 +144 +6 +unpickleExprV2 +Apply +false +0 +false +new VisitorType[T]{} + +2 +macro-late-suspend/VisitorMacros.scala +example +VisitorMacros$package +Object +example.VisitorMacros$package +mkVisitorTypeImpl +40 +69 +5 +mkVisitorTypeImpl +DefDef +false +0 +false +private def mkVisitorTypeImpl + +3 +macro-late-suspend/Test.scala +example +Test +Object +example.Test + +102 +121 +8 + +Apply +false +0 +false +mkVisitorType[Test] + +4 +macro-late-suspend/UsesTest.scala +example +UsesTest$package +Object +example.UsesTest$package + +22 +22 +3 + +Literal +true +0 +false + + From f7b2aa54e7c77aa2c095ef364dddcc43cbaffd6a Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Fri, 10 Jan 2025 11:32:46 +0100 Subject: [PATCH 4/4] Initialise Coverage object only when the option is used --- compiler/src/dotty/tools/dotc/core/Contexts.scala | 7 +++---- .../tools/dotc/transform/InstrumentCoverage.scala | 12 ++++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index d6d726c8e92d..7c54d1392720 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -983,11 +983,10 @@ object Contexts { /** Was best effort file used during compilation? */ private[core] var usedBestEffortTasty = false - /** Stores all instrumented statements (for InstrumentCoverage). - * We need this information to be persisted across different runs, - * so it's stored here. + /** If coverage option is used, it stores all instrumented statements (for InstrumentCoverage). + * We need this information to be persisted across different runs, so it's stored here. */ - private[dotc] var coverage: Coverage = Coverage() + private[dotc] var coverage: Coverage | Null = null // Types state /** A table for hash consing unique types */ diff --git a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala index 8a8cbf037dc0..0229284a1b5f 100644 --- a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala +++ b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala @@ -55,13 +55,17 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: .foreach(_.nn.delete()) end if + // Initialise a coverage object if it does not exist yet + if ctx.base.coverage == null then + ctx.base.coverage = Coverage() + coverageExcludeClasslikePatterns = ctx.settings.coverageExcludeClasslikes.value.map(_.r.pattern) coverageExcludeFilePatterns = ctx.settings.coverageExcludeFiles.value.map(_.r.pattern) - ctx.base.coverage.removeStatementsFromFile(ctx.compilationUnit.source.file.absolute.jpath) + ctx.base.coverage.nn.removeStatementsFromFile(ctx.compilationUnit.source.file.absolute.jpath) super.run - Serializer.serialize(ctx.base.coverage, outputPath, ctx.settings.sourceroot.value) + Serializer.serialize(ctx.base.coverage.nn, outputPath, ctx.settings.sourceroot.value) private def isClassIncluded(sym: Symbol)(using Context): Boolean = val fqn = sym.fullName.toText(ctx.printerFn(ctx)).show @@ -105,7 +109,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: * @return the statement's id */ private def recordStatement(tree: Tree, pos: SourcePosition, branch: Boolean)(using ctx: Context): Int = - val id = ctx.base.coverage.nextStatementId() + val id = ctx.base.coverage.nn.nextStatementId() val sourceFile = pos.source val statement = Statement( @@ -121,7 +125,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: treeName = tree.getClass.getSimpleName.nn, branch ) - ctx.base.coverage.addStatement(statement) + ctx.base.coverage.nn.addStatement(statement) id /**