Skip to content

Commit

Permalink
Don't lift standard string interpolators for coverage + add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
TheElectronWill committed Jul 2, 2022
1 parent ee4a6ce commit 1fb2b7a
Show file tree
Hide file tree
Showing 7 changed files with 670 additions and 26 deletions.
26 changes: 20 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
3 changes: 3 additions & 0 deletions tests/coverage/run/interpolation/test.check
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@
2: t
3: t
4: y
1, 3
0x007f
a\nb
8 changes: 7 additions & 1 deletion tests/coverage/run/interpolation/test.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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")
174 changes: 155 additions & 19 deletions tests/coverage/run/interpolation/test.scoverage.check
Original file line number Diff line number Diff line change
Expand Up @@ -75,78 +75,214 @@ interpolation/test.scala
Test$
Object
<empty>.Test$
main
147
176
hexa
113
126
6
f
Apply
false
0
false
f"0x${i}%04x"

4
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
hexa
82
90
5
hexa
DefDef
false
0
false
def hexa

5
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
main
195
224
9
apply
Apply
false
0
false
List("d", "o", "t", "t", "y")

4
6
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
$anonfun
220
229
8
267
276
10
s
Apply
false
0
false
s"$i: $s"

5
7
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
$anonfun
212
230
8
259
277
10
println
Apply
false
0
false
println(s"$i: $s")

6
8
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
main
182
231
8
229
278
10
map
Apply
false
0
false
xs.zipWithIndex.map((s, i) => println(s"$i: $s"))

7
9
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
main
82
90
5
292
308
12
simple
Apply
false
0
false
simple(1, "abc")

10
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
main
284
309
12
println
Apply
false
0
false
println(simple(1, "abc"))

11
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
main
322
331
13
hexa
Apply
false
0
false
hexa(127)

12
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
main
314
332
13
println
Apply
false
0
false
println(hexa(127))

13
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
main
345
354
14
raw
Apply
false
0
false
raw"a\nb"

14
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
main
337
355
14
println
Apply
false
0
false
println(raw"a\nb")

15
interpolation/test.scala
<empty>
Test$
Object
<empty>.Test$
main
130
138
8
main
DefDef
false
Expand Down
3 changes: 3 additions & 0 deletions tests/coverage/run/lifting-bool/test.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
true false true false false
true
true
19 changes: 19 additions & 0 deletions tests/coverage/run/lifting-bool/test.scala
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 1fb2b7a

Please sign in to comment.