diff --git a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala index 29572a4ae30d..ad4ca5649653 100644 --- a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala +++ b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala @@ -2,20 +2,20 @@ package dotty.tools.dotc package transform import java.io.File - import ast.tpd.* + import collection.mutable import core.Flags.* import core.Contexts.{Context, ctx, inContext} import core.DenotTransformers.IdentityDenotTransformer -import core.Symbols.{defn, Symbol} +import core.Symbols.{Symbol, defn} import core.Constants.Constant import core.NameOps.isContextFunction import core.StdNames.nme import core.Types.* import coverage.* import typer.LiftCoverage -import util.{SourcePosition, SourceFile} +import util.{SourceFile, SourcePosition} import util.Spans.Span import localopt.StringInterpolatorOpt import inlines.Inlines @@ -120,6 +120,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: */ private def createInvokeCall(tree: Tree, pos: SourcePosition, branch: Boolean = false)(using Context): Apply = val statementId = recordStatement(tree, pos, branch) + println(s"createInvokeCall: $statementId") val span = pos.span.toSynthetic invokeCall(statementId, span) @@ -132,6 +133,8 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: * @return instrumentation result, with the preparation statement, coverage call and tree separated */ private def tryInstrument(tree: Apply)(using Context): InstrumentedParts = + println(s"tryInstrument Tree: ${tree}") + if canInstrumentApply(tree) then // Create a call to Invoker.invoked(coverageDirectory, newStatementId) val coverageCall = createInvokeCall(tree, tree.sourcePos) @@ -156,25 +159,33 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: val transformed = cpy.Apply(tree)(transform(tree.fun), transform(tree.args)) InstrumentedParts.notCovered(transformed) - private def tryInstrument(tree: Ident)(using Context): InstrumentedParts = + private def tryInstrument(tree: Ident)(using Context): InstrumentedParts = { + println(s"tryInstrument Ident: ${tree}") + + val sym = tree.symbol - if canInstrumentParameterless(sym) then + + if (canInstrumentParameterless(sym)) { // call to a local parameterless method f val coverageCall = createInvokeCall(tree, tree.sourcePos) InstrumentedParts.singleExpr(coverageCall, tree) - else - InstrumentedParts.notCovered(tree) + } else { + println(s"tryInstrument Ident: ${tree} not instrumented") + InstrumentedParts.notCovered(tree) + } + } - private def tryInstrument(tree: Select)(using Context): InstrumentedParts = + private def tryInstrument(tree: Select)(using Context): InstrumentedParts = { val sym = tree.symbol val transformed = cpy.Select(tree)(transform(tree.qualifier), tree.name) + println(s"tryInstrument Select: ${tree}") if canInstrumentParameterless(sym) then // call to a parameterless method val coverageCall = createInvokeCall(tree, tree.sourcePos) InstrumentedParts.singleExpr(coverageCall, transformed) else InstrumentedParts.notCovered(transformed) - + } /** Generic tryInstrument */ private def tryInstrument(tree: Tree)(using Context): InstrumentedParts = tree match @@ -234,6 +245,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: case TypeApply(fun, args) => // Here is where `InstrumentedParts` becomes useful! // We extract its components and act carefully. + println(s"TypeApply: ${fun}") val InstrumentedParts(pre, coverageCall, expr) = tryInstrument(fun) if coverageCall.isEmpty then @@ -444,6 +456,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: * should not be changed to {val $x = f(); T($x)}(1) but to {val $x = f(); val $y = 1; T($x)($y)} */ private def needsLift(tree: Apply)(using Context): Boolean = + println(s"start needsLift: $tree") def isShortCircuitedOp(sym: Symbol) = sym == defn.Boolean_&& || sym == defn.Boolean_|| @@ -465,8 +478,10 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: case a: Apply => needsLift(a) case _ => false - nestedApplyNeedsLift || + val liftMe = nestedApplyNeedsLift || !isUnliftableFun(fun) && !tree.args.isEmpty && !tree.args.forall(LiftCoverage.noLift) + println(s"end needsLift: $tree, liftMe: $liftMe, nestedApplyNeedsLift: $nestedApplyNeedsLift, isUnliftableFun: ${isUnliftableFun(fun)}, tree.args.isEmpty: ${tree.args}, tree.args.forall(LiftCoverage.noLift): ${tree.args.forall(LiftCoverage.noLift)}") + liftMe /** Check if an Apply can be instrumented. Prevents this phase from generating incorrect code. */ private def canInstrumentApply(tree: Apply)(using Context): Boolean = @@ -475,10 +490,10 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: case _ => false val sym = tree.symbol - !sym.isOneOf(ExcludeMethodFlags) + val canI = !sym.isOneOf(ExcludeMethodFlags) && !isCompilerIntrinsicMethod(sym) && !(sym.isClassConstructor && isSecondaryCtorDelegateCall) - && (tree.typeOpt match + && (tree.typeOpt match { case AppliedType(tycon: NamedType, _) => /* If the last expression in a block is a context function, we'll try to summon its arguments at the current point, even if the expected type @@ -501,19 +516,25 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer: */ false case _ => - true - ) + false + }) + println(s"canInstrumentApply ${tree.symbol}: $canI is ${sym.name} is method ${sym.is(Method)} is compiler intrinsic ${isCompilerIntrinsicMethod(sym)} typeOpt ${tree.typeOpt}") + canI + /** Is this the symbol of a parameterless method that we can instrument? * Note: it is crucial that `asInstanceOf` and `isInstanceOf`, among others, * do NOT get instrumented, because that would generate invalid code and crash * in post-erasure checking. */ - private def canInstrumentParameterless(sym: Symbol)(using Context): Boolean = - sym.is(Method, butNot = ExcludeMethodFlags) + private def canInstrumentParameterless(sym: Symbol)(using Context): Boolean = { + val isP = (sym.is(Method, butNot = ExcludeMethodFlags) && sym.info.isParameterless && !isCompilerIntrinsicMethod(sym) - && !sym.info.typeSymbol.name.isContextFunction // exclude context functions like in canInstrumentApply + && !sym.info.typeSymbol.name.isContextFunction ) // exclude context functions like in canInstrumentApply) + println(s"canInstrumentParameterless $sym: $isP is ${sym.name} kind string ${sym.kindString} is method ${sym.is(Method)} is parameterless ${sym.info.isParameterless} is compiler intrinsic ${isCompilerIntrinsicMethod(sym)} typeSymbol ${sym.info.typeSymbol} is context function ${sym.info.typeSymbol.name.isContextFunction}") + isP + } /** Does sym refer to a "compiler intrinsic" method, which only exist during compilation, * like Any.isInstanceOf? diff --git a/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala b/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala index 77e172f61167..90bced7834cf 100644 --- a/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala +++ b/compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala @@ -23,7 +23,7 @@ class CoverageTests: private val scalaFile = FileSystems.getDefault.getPathMatcher("glob:**.scala") private val rootSrc = Paths.get(userDir, "tests", "coverage") - + @Test def checkCoverageStatements(): Unit = checkCoverageIn(rootSrc.resolve("pos"), false) diff --git a/tests/coverage/pos/Escaping.scoverage.check b/tests/coverage/pos/Escaping.scoverage.check index ecb907a9d222..b349e80e239a 100644 --- a/tests/coverage/pos/Escaping.scoverage.check +++ b/tests/coverage/pos/Escaping.scoverage.check @@ -42,6 +42,23 @@ covtest.\n Class covtest.\n.\r\n\f \r\n\f +69 +73 +3 +\\ +Ident +false +0 +false +`\\\\` + +2 +Escaping.scala +covtest.\n +\r\n\f +Class +covtest.\n.\r\n\f +\r\n\f 40 48 3 diff --git a/tests/coverage/pos/ForComprehension.scala b/tests/coverage/pos/ForComprehension.scala new file mode 100644 index 000000000000..c774bbbdde19 --- /dev/null +++ b/tests/coverage/pos/ForComprehension.scala @@ -0,0 +1,10 @@ +package covtest + +def testForComprehension: Unit = + for { + a <- List(1) + b <- List(1) + if b > 1 + c = a + b + } yield (a, b, c) + diff --git a/tests/coverage/pos/ForComprehension.scoverage.check b/tests/coverage/pos/ForComprehension.scoverage.check new file mode 100644 index 000000000000..65f463e01ae0 --- /dev/null +++ b/tests/coverage/pos/ForComprehension.scoverage.check @@ -0,0 +1,190 @@ +# 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 +ForComprehension.scala +covtest +ForComprehension$package$ +Object +covtest.ForComprehension$package$ +testForComprehension +52 +138 +3 +flatMap +Apply +false +0 +false +for {\n a <- List(1)\n b <- List(1)\n if b > 1\n c = a + b\n } yield (a, b, c) + +1 +ForComprehension.scala +covtest +ForComprehension$package$ +Object +covtest.ForComprehension$package$ +testForComprehension +67 +74 +4 +apply +Apply +false +0 +false +List(1) + +2 +ForComprehension.scala +covtest +ForComprehension$package$ +Object +covtest.ForComprehension$package$ +testForComprehension +67 +71 +4 +List +Ident +false +0 +false +List + +3 +ForComprehension.scala +covtest +ForComprehension$package$ +Object +covtest.ForComprehension$package$ +$anonfun +79 +138 +5 +map +Apply +false +0 +false +b <- List(1)\n if b > 1\n c = a + b\n } yield (a, b, c) + +4 +ForComprehension.scala +covtest +ForComprehension$package$ +Object +covtest.ForComprehension$package$ +$anonfun +79 +118 +5 +map +Apply +false +0 +false +b <- List(1)\n if b > 1\n c = a + b + +5 +ForComprehension.scala +covtest +ForComprehension$package$ +Object +covtest.ForComprehension$package$ +$anonfun +79 +104 +5 +withFilter +Apply +false +0 +false +b <- List(1)\n if b > 1 + +6 +ForComprehension.scala +covtest +ForComprehension$package$ +Object +covtest.ForComprehension$package$ +$anonfun +84 +91 +5 +apply +Apply +false +0 +false +List(1) + +7 +ForComprehension.scala +covtest +ForComprehension$package$ +Object +covtest.ForComprehension$package$ +$anonfun +84 +88 +5 +List +Ident +false +0 +false +List + +8 +ForComprehension.scala +covtest +ForComprehension$package$ +Object +covtest.ForComprehension$package$ +$anonfun +79 +110 +5 +apply +Apply +false +0 +false +b <- List(1)\n if b > 1\n c + +9 +ForComprehension.scala +covtest +ForComprehension$package$ +Object +covtest.ForComprehension$package$ +testForComprehension +17 +41 +2 +testForComprehension +DefDef +false +0 +false +def testForComprehension + diff --git a/tests/coverage/pos/Lift.scoverage.check b/tests/coverage/pos/Lift.scoverage.check index a1c656cbdb67..a5477d3d8761 100644 --- a/tests/coverage/pos/Lift.scoverage.check +++ b/tests/coverage/pos/Lift.scoverage.check @@ -76,6 +76,23 @@ SomeFunctions Class covtest.SomeFunctions c +83 +96 +5 + +Select +false +0 +false +SomeFunctions + +4 +Lift.scala +covtest +SomeFunctions +Class +covtest.SomeFunctions +c 75 80 5 @@ -86,7 +103,7 @@ false false def c -4 +5 Lift.scala covtest SomeFunctions @@ -103,7 +120,7 @@ false false c.f(g()) -5 +6 Lift.scala covtest SomeFunctions @@ -120,7 +137,24 @@ false false c -6 +7 +Lift.scala +covtest +SomeFunctions +Class +covtest.SomeFunctions +test +113 +116 +7 +f +Select +false +0 +false +c.f + +8 Lift.scala covtest SomeFunctions @@ -137,7 +171,24 @@ false false g() +9 +Lift.scala +covtest +SomeFunctions +Class +covtest.SomeFunctions +test +117 +118 7 +g +Select +false +0 +false +g + +10 Lift.scala covtest SomeFunctions diff --git a/tests/coverage/run/for_comprehension/test.check b/tests/coverage/run/for_comprehension/test.check new file mode 100644 index 000000000000..1191247b6d9a --- /dev/null +++ b/tests/coverage/run/for_comprehension/test.check @@ -0,0 +1,2 @@ +1 +2 diff --git a/tests/coverage/run/for_comprehension/test.scala b/tests/coverage/run/for_comprehension/test.scala new file mode 100644 index 000000000000..4c8260f692c2 --- /dev/null +++ b/tests/coverage/run/for_comprehension/test.scala @@ -0,0 +1,41 @@ +@main +def Test: Unit = { +// +// def unreachableFunction(): Seq[Int] = { +// for { +// a <- List(1) +// b <- List(2) +// } yield { +// println(a) +// println(b) +// a + b +// } +// } +// +// def unreachableFunctionUnlessTrue(flag: Boolean): Option[Int] = { +// if (flag) { +// val foo: Seq[Int] = for { +// a <- List(1) +// b <- List(2) +// } yield { +// println(a) +// println(b) +// a + b +// } +// foo.headOption +// } else { +// None +// } +// } + + for { + a <- List(1) + b <- List(2) + } yield { + println(a) + println(b) + (a, b) + } + +// unreachableFunctionUnlessTrue(false) +} diff --git a/tests/coverage/run/for_comprehension/test.scoverage.check b/tests/coverage/run/for_comprehension/test.scoverage.check new file mode 100644 index 000000000000..c4434057055f --- /dev/null +++ b/tests/coverage/run/for_comprehension/test.scoverage.check @@ -0,0 +1,275 @@ +# 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 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +Test +27 +124 +2 +flatMap +Apply +false +0 +false +for {\n a <- List(1)\n b <- List(2)\n } yield {\n println(a)\n println(b)\n (a, b)\n } + +1 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +Test +42 +49 +3 +apply +Apply +false +0 +false +List(1) + +2 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +Test +42 +46 +3 +List +Ident +false +0 +false +List + +3 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +54 +124 +4 +map +Apply +false +0 +false +b <- List(2)\n } yield {\n println(a)\n println(b)\n (a, b)\n } + +4 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +59 +66 +4 +apply +Apply +false +0 +false +List(2) + +5 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +59 +63 +4 +List +Ident +false +0 +false +List + +6 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +84 +94 +6 +println +Apply +false +0 +false +println(a) + +7 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +84 +91 +6 +println +Ident +false +0 +false +println + +8 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +92 +93 +6 +a +Ident +false +0 +false +a + +9 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +99 +109 +7 +println +Apply +false +0 +false +println(b) + +10 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +99 +106 +7 +println +Ident +false +0 +false +println + +11 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +107 +108 +7 +b +Ident +false +0 +false +b + +12 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +115 +116 +8 +a +Ident +false +0 +false +a + +13 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +$anonfun +118 +119 +8 +b +Ident +false +0 +false +b + +14 +for_comprehension/test.scala + +test$package$ +Object +.test$package$ +Test +0 +14 +1 +Test +DefDef +false +0 +false +@main\ndef Test + diff --git a/tests/coverage/run/inline-def/test.scoverage.check b/tests/coverage/run/inline-def/test.scoverage.check index 784c0a00b62b..b028c734f2f9 100644 --- a/tests/coverage/run/inline-def/test.scoverage.check +++ b/tests/coverage/run/inline-def/test.scoverage.check @@ -42,6 +42,23 @@ test$package$ Object .test$package$ Test +225 +226 +12 + +Select +false +0 +false +A + +2 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test 231 243 13 @@ -52,7 +69,24 @@ false false println(a.x) -2 +3 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +239 +242 +13 +x +Select +false +0 +false +a.x + +4 inline-def/test.scala test$package$ @@ -69,7 +103,7 @@ false false println(a.foo) -3 +5 inline-def/test.scala test$package$ @@ -86,7 +120,24 @@ false false "foo".toString -4 +6 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +134 +148 +7 +toString +Select +false +0 +false +"foo".toString + +7 inline-def/test.scala test$package$ @@ -103,7 +154,7 @@ false false println(a.bar) -5 +8 inline-def/test.scala test$package$ @@ -120,7 +171,24 @@ false false "bar".toString -6 +9 +inline-def/test.scala + +test$package$ +Object +.test$package$ +Test +176 +190 +8 +toString +Select +false +0 +false +"bar".toString + +10 inline-def/test.scala test$package$ @@ -137,7 +205,7 @@ false false println(b.foo) -7 +11 inline-def/test.scala test$package$ @@ -154,7 +222,7 @@ false false b.foo -8 +12 inline-def/test.scala test$package$ diff --git a/tests/coverage/run/java-methods/test.scoverage.check b/tests/coverage/run/java-methods/test.scoverage.check index 7d3752c8db20..7e5f48056b5a 100644 --- a/tests/coverage/run/java-methods/test.scoverage.check +++ b/tests/coverage/run/java-methods/test.scoverage.check @@ -42,6 +42,23 @@ test$package$ Object .test$package$ Test +61 +81 +4 +simple +Select +false +0 +false +StaticMethods.simple + +2 +java-methods/test.scala + +test$package$ +Object +.test$package$ +Test 86 127 5 @@ -52,7 +69,24 @@ false false StaticMethods.withTypeParam[Any](a => ()) -2 +3 +java-methods/test.scala + +test$package$ +Object +.test$package$ +Test +86 +113 +5 +withTypeParam +Select +false +0 +false +StaticMethods.withTypeParam + +4 java-methods/test.scala test$package$ @@ -69,7 +103,24 @@ false false JavaObject() -3 +5 +java-methods/test.scala + +test$package$ +Object +.test$package$ +Test +140 +150 +6 + +Select +false +0 +false +JavaObject + +6 java-methods/test.scala test$package$ @@ -86,7 +137,24 @@ false false obj.f() -4 +7 +java-methods/test.scala + +test$package$ +Object +.test$package$ +Test +155 +160 +7 +f +Select +false +0 +false +obj.f + +8 java-methods/test.scala test$package$ @@ -103,7 +171,7 @@ false false println(obj.identity[Int](0)) -5 +9 java-methods/test.scala test$package$ @@ -120,7 +188,24 @@ false false obj.identity[Int](0) -6 +10 +java-methods/test.scala + +test$package$ +Object +.test$package$ +Test +173 +185 +8 +identity +Select +false +0 +false +obj.identity + +11 java-methods/test.scala test$package$ @@ -137,7 +222,7 @@ false false println("ok!") -7 +12 java-methods/test.scala test$package$