Fix liftForCoverage, in particular for erased arguments

Fixes scala#15505

compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala

@@ -4,6 +4,7 @@ package transform
 import java.io.File
 import java.util.concurrent.atomic.AtomicInteger
 
+import ast.tpd.*
 import collection.mutable
 import core.Flags.*
 import core.Contexts.{Context, ctx, inContext}
@@ -23,7 +24,6 @@ import coverage.*
   * The result can then be consumed by the Scoverage tool.
   */
 class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
-  import ast.tpd._
 
   override def phaseName = InstrumentCoverage.name
 
@@ -60,7 +60,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
 
   /** Transforms trees to insert calls to Invoker.invoked to compute the coverage when the code is called */
   private class CoverageTransformer extends Transformer:
-    override def transform(tree: Tree)(using ctx: Context): Tree =
+    override def transform(tree: Tree)(using Context): Tree =
       inContext(transformCtx(tree)) { // necessary to position inlined code properly
         tree match
           // simple cases
@@ -280,10 +280,6 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
         sym.exists &&
         sym == defn.Boolean_&& || sym == defn.Boolean_||
 
-      def isContextual(fun: Apply): Boolean =
-        val args = fun.args
-        args.nonEmpty && args.head.symbol.isAllOf(Given | Implicit)
-
       val fun = tree.fun
       val nestedApplyNeedsLift = fun match
         case a: Apply => needsLift(a)

compiler/src/dotty/tools/dotc/typer/EtaExpansion.scala

@@ -159,23 +159,39 @@ class LiftComplex extends Lifter {
 }
 object LiftComplex extends LiftComplex
 
-/** Lift complex + lift the prefixes */
-object LiftCoverage extends LiftComplex {
+/** Lift impure + lift the prefixes */
+object LiftCoverage extends LiftImpure {
 
-  private val LiftEverything = new Property.Key[Boolean]
+  // Property indicating whether we're currently lifting the arguments of an application
+  private val LiftingArgs = new Property.Key[Boolean]
 
-  private inline def liftEverything(using Context): Boolean =
-    ctx.property(LiftEverything).contains(true)
+  private inline def liftingArgs(using Context): Boolean =
+    ctx.property(LiftingArgs).contains(true)
 
-  private def liftEverythingContext(using Context): Context =
-    ctx.fresh.setProperty(LiftEverything, true)
+  private def liftingArgsContext(using Context): Context =
+    ctx.fresh.setProperty(LiftingArgs, true)
+
+  /** Variant of `noLift` for the arguments of applications.
+   *  To produce the right coverage information (especially in case of exceptions), we must lift:
+   *  - all the applications, except the erased ones
+   *  - all the impure arguments
+   *
+   * There's no need to lift the other arguments.
+   */
+  private def noLiftArg(arg: tpd.Tree)(using Context): Boolean =
+    arg match
+      case a: tpd.Apply => a.symbol.is(Erased) // don't lift erased applications, but lift all others
+      case tpd.Block(stats, expr) => stats.forall(noLiftArg) && noLiftArg(expr)
+      case tpd.Inlined(_, bindings, expr) => noLiftArg(expr)
+      case tpd.Typed(expr, _) => noLiftArg(expr)
+      case _ => super.noLift(arg)
 
   override def noLift(expr: tpd.Tree)(using Context) =
-    !liftEverything && super.noLift(expr)
+    if liftingArgs then noLiftArg(expr) else super.noLift(expr)
 
   def liftForCoverage(defs: mutable.ListBuffer[tpd.Tree], tree: tpd.Apply)(using Context) = {
     val liftedFun = liftApp(defs, tree.fun)
-    val liftedArgs = liftArgs(defs, tree.fun.tpe, tree.args)(using liftEverythingContext)
+    val liftedArgs = liftArgs(defs, tree.fun.tpe, tree.args)(using liftingArgsContext)
     tpd.cpy.Apply(tree)(liftedFun, liftedArgs)
   }
 }

tests/coverage/pos/ContextFunctions.scoverage.check

@@ -92,16 +92,16 @@ covtest
 Imperative
 Class
 covtest.Imperative
-$anonfun
-267
-294
+readPerson
+252
+295
 13
 invoked
 Apply
 false
 0
 false
-readName2(using e)(using s)
+OnError((e) => readName2(using e)(using s))
 
 5
 ContextFunctions.scala
@@ -113,7 +113,7 @@ $anonfun
 267
 294
 13
-apply
+invoked
 Apply
 false
 0
@@ -126,16 +126,16 @@ covtest
 Imperative
 Class
 covtest.Imperative
-readPerson
-252
-295
+$anonfun
+267
+294
 13
-invoked
+apply
 Apply
 false
 0
 false
-OnError((e) => readName2(using e)(using s))
+readName2(using e)(using s)
 
 7
 ContextFunctions.scala

tests/coverage/run/erased/test.check

@@ -0,0 +1,3 @@
+foo(a)(b)
+identity(idem)
+foo(a)(idem)

tests/coverage/run/erased/test.scala

@@ -0,0 +1,15 @@
+import scala.language.experimental.erasedDefinitions
+
+erased def e(x: String): String = "x"
+def foo(erased a: String)(b: String): String =
+  println(s"foo(a)($b)")
+  b
+
+def identity(s: String): String =
+  println(s"identity($s)")
+  s
+
+@main
+def Test: Unit =
+  foo(e("a"))("b")
+  foo(e("a"))(identity("idem"))

tests/coverage/run/erased/test.scoverage.check

@@ -0,0 +1,225 @@
+# 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
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+e
+54
+66
+2
+e
+DefDef
+false
+0
+false
+erased def e
+
+1
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+foo
+149
+162
+4
+s
+Apply
+false
+0
+false
+s"foo(a)($b)"
+
+2
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+foo
+141
+163
+4
+println
+Apply
+false
+0
+false
+println(s"foo(a)($b)")
+
+3
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+foo
+92
+99
+3
+foo
+DefDef
+false
+0
+false
+def foo
+
+4
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+identity
+213
+228
+8
+s
+Apply
+false
+0
+false
+s"identity($s)"
+
+5
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+identity
+205
+229
+8
+println
+Apply
+false
+0
+false
+println(s"identity($s)")
+
+6
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+identity
+169
+181
+7
+identity
+DefDef
+false
+0
+false
+def identity
+
+7
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+Test
+264
+270
+13
+e
+Apply
+false
+0
+false
+e("a")
+
+8
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+Test
+260
+276
+13
+foo
+Apply
+false
+0
+false
+foo(e("a"))("b")
+
+9
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+Test
+291
+307
+14
+identity
+Apply
+false
+0
+false
+identity("idem")
+
+10
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+Test
+279
+308
+14
+foo
+Apply
+false
+0
+false
+foo(e("a"))(identity("idem"))
+
+11
+erased/test.scala
+<empty>
+test$package$
+Object
+<empty>.test$package$
+Test
+235
+249
+12
+Test
+DefDef
+false
+0
+false
+@main
+def Test
+