diff --git a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala index 9944cd2c68b7..86c1c0b8274f 100644 --- a/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala +++ b/compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala @@ -273,20 +273,34 @@ 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 = - def isBooleanOperator(fun: Tree) = - // We don't want to lift a || getB(), to avoid calling getB if a is true. - // Same idea with a && getB(): if a is false, getB shouldn't be called. - val sym = fun.symbol - sym.exists && + inline def isShortCircuitedOp(sym: Symbol) = sym == defn.Boolean_&& || sym == defn.Boolean_|| + inline def isCompilerIntrinsic(sym: Symbol) = + sym == defn.StringContext_s || + sym == defn.StringContext_f || + sym == defn.StringContext_raw + + def isUnliftableFun(fun: Tree) = + /* + * We don't want to lift a || getB(), to avoid calling getB if a is true. + * Same idea with a && getB(): if a is false, getB shouldn't be called. + * + * On top of that, the `s`, `f` and `raw` string interpolators are special-cased + * by the compiler and will disappear in phase StringInterpolatorOpt, therefore + * they shouldn't be lifted. + */ + val sym = fun.symbol + sym.exists && (isShortCircuitedOp(sym) || isCompilerIntrinsic(sym)) + end + val fun = tree.fun val nestedApplyNeedsLift = fun match case a: Apply => needsLift(a) case _ => false nestedApplyNeedsLift || - !isBooleanOperator(fun) && !tree.args.isEmpty && !tree.args.forall(LiftCoverage.noLift) + !isUnliftableFun(fun) && !tree.args.isEmpty && !tree.args.forall(LiftCoverage.noLift) /** Check if the body of a DefDef can be instrumented with instrumentBody. */ private def canInstrumentDefDef(tree: DefDef)(using Context): Boolean = diff --git a/tests/coverage/run/interpolation/test.check b/tests/coverage/run/interpolation/test.check index 9ce4b367db49..34408817a623 100644 --- a/tests/coverage/run/interpolation/test.check +++ b/tests/coverage/run/interpolation/test.check @@ -3,3 +3,6 @@ 2: t 3: t 4: y +1, 3 +0x007f +a\nb diff --git a/tests/coverage/run/interpolation/test.scala b/tests/coverage/run/interpolation/test.scala index 3745367b593d..183b1131b22b 100644 --- a/tests/coverage/run/interpolation/test.scala +++ b/tests/coverage/run/interpolation/test.scala @@ -3,7 +3,13 @@ object Test: def simple(a: Int, b: String): String = s"$a, ${b.length}" + def hexa(i: Int): String = + f"0x${i}%04x" + def main(args: Array[String]): Unit = val xs: List[String] = List("d", "o", "t", "t", "y") - xs.zipWithIndex.map((s, i) => println(s"$i: $s")) + + println(simple(1, "abc")) + println(hexa(127)) + println(raw"a\nb") diff --git a/tests/coverage/run/interpolation/test.scoverage.check b/tests/coverage/run/interpolation/test.scoverage.check index 9c93d58f182c..f30b391d20e5 100644 --- a/tests/coverage/run/interpolation/test.scoverage.check +++ b/tests/coverage/run/interpolation/test.scoverage.check @@ -75,10 +75,44 @@ interpolation/test.scala Test$ Object .Test$ -main -147 -176 +hexa +113 +126 6 +f +Apply +false +0 +false +f"0x${i}%04x" + +4 +interpolation/test.scala + +Test$ +Object +.Test$ +hexa +82 +90 +5 +hexa +DefDef +false +0 +false +def hexa + +5 +interpolation/test.scala + +Test$ +Object +.Test$ +main +195 +224 +9 apply Apply false @@ -86,16 +120,16 @@ false false List("d", "o", "t", "t", "y") -4 +6 interpolation/test.scala Test$ Object .Test$ $anonfun -220 -229 -8 +267 +276 +10 s Apply false @@ -103,16 +137,16 @@ false false s"$i: $s" -5 +7 interpolation/test.scala Test$ Object .Test$ $anonfun -212 -230 -8 +259 +277 +10 println Apply false @@ -120,16 +154,16 @@ false false println(s"$i: $s") -6 +8 interpolation/test.scala Test$ Object .Test$ main -182 -231 -8 +229 +278 +10 map Apply false @@ -137,16 +171,118 @@ false false xs.zipWithIndex.map((s, i) => println(s"$i: $s")) -7 +9 interpolation/test.scala Test$ Object .Test$ main -82 -90 -5 +292 +308 +12 +simple +Apply +false +0 +false +simple(1, "abc") + +10 +interpolation/test.scala + +Test$ +Object +.Test$ +main +284 +309 +12 +println +Apply +false +0 +false +println(simple(1, "abc")) + +11 +interpolation/test.scala + +Test$ +Object +.Test$ +main +322 +331 +13 +hexa +Apply +false +0 +false +hexa(127) + +12 +interpolation/test.scala + +Test$ +Object +.Test$ +main +314 +332 +13 +println +Apply +false +0 +false +println(hexa(127)) + +13 +interpolation/test.scala + +Test$ +Object +.Test$ +main +345 +354 +14 +raw +Apply +false +0 +false +raw"a\nb" + +14 +interpolation/test.scala + +Test$ +Object +.Test$ +main +337 +355 +14 +println +Apply +false +0 +false +println(raw"a\nb") + +15 +interpolation/test.scala + +Test$ +Object +.Test$ +main +130 +138 +8 main DefDef false diff --git a/tests/coverage/run/lifting-bool/test.check b/tests/coverage/run/lifting-bool/test.check new file mode 100644 index 000000000000..c388c06b06fb --- /dev/null +++ b/tests/coverage/run/lifting-bool/test.check @@ -0,0 +1,3 @@ +true false true false false +true +true \ No newline at end of file diff --git a/tests/coverage/run/lifting-bool/test.scala b/tests/coverage/run/lifting-bool/test.scala new file mode 100644 index 000000000000..f03a01fae60b --- /dev/null +++ b/tests/coverage/run/lifting-bool/test.scala @@ -0,0 +1,19 @@ + +def notCalled() = ??? + +def f(x: Boolean, y: Boolean): Boolean = x + +@main +def Test: Unit = + val a = true || notCalled() // true + val b = false && notCalled() // false + val c = (true || false) || notCalled() // true + val d = true && (false && notCalled()) // false + val e = (true && false) && notCalled() // false + println(s"$a $b $c $d $e") + + var x = f(true, false) + println(x) // true + + x = f(true || notCalled(), false && notCalled()) + println(x) // true diff --git a/tests/coverage/run/lifting-bool/test.scoverage.check b/tests/coverage/run/lifting-bool/test.scoverage.check new file mode 100644 index 000000000000..49f83b4eecc8 --- /dev/null +++ b/tests/coverage/run/lifting-bool/test.scoverage.check @@ -0,0 +1,463 @@ +# 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 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +notCalled +1 +14 +1 +notCalled +DefDef +false +0 +false +def notCalled + +1 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +f +24 +29 +3 +f +DefDef +false +0 +false +def f + +2 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +109 +120 +7 +notCalled +Apply +false +0 +false +notCalled() + +3 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +101 +120 +7 +|| +Apply +false +0 +false +true || notCalled() + +4 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +159 +170 +8 +notCalled +Apply +false +0 +false +notCalled() + +5 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +150 +170 +8 +&& +Apply +false +0 +false +false && notCalled() + +6 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +201 +214 +9 +|| +Apply +false +0 +false +true || false + +7 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +219 +230 +9 +notCalled +Apply +false +0 +false +notCalled() + +8 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +200 +230 +9 +|| +Apply +false +0 +false +(true || false) || notCalled() + +9 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +267 +278 +10 +notCalled +Apply +false +0 +false +notCalled() + +10 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +258 +278 +10 +&& +Apply +false +0 +false +false && notCalled() + +11 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +249 +278 +10 +&& +Apply +false +0 +false +true && (false && notCalled() + +12 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +300 +313 +11 +&& +Apply +false +0 +false +true && false + +13 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +318 +329 +11 +notCalled +Apply +false +0 +false +notCalled() + +14 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +299 +329 +11 +&& +Apply +false +0 +false +(true && false) && notCalled() + +15 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +349 +366 +12 +s +Apply +false +0 +false +s"$a $b $c $d $e" + +16 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +341 +367 +12 +println +Apply +false +0 +false +println(s"$a $b $c $d $e") + +17 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +379 +393 +14 +f +Apply +false +0 +false +f(true, false) + +18 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +396 +406 +15 +println +Apply +false +0 +false +println(x) + +19 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +432 +443 +17 +notCalled +Apply +false +0 +false +notCalled() + +20 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +424 +443 +17 +|| +Apply +false +0 +false +true || notCalled() + +21 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +454 +465 +17 +notCalled +Apply +false +0 +false +notCalled() + +22 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +445 +465 +17 +&& +Apply +false +0 +false +false && notCalled() + +23 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +422 +466 +17 +f +Apply +false +0 +false +f(true || notCalled(), false && notCalled()) + +24 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +469 +479 +18 +println +Apply +false +0 +false +println(x) + +25 +lifting-bool/test.scala + +test$package$ +Object +.test$package$ +Test +68 +82 +6 +Test +DefDef +false +0 +false +@main +def Test +