Fix lifting of arguments with -coverage-out

[TheElectronWill]

Fixes #15078, fixes #15487, fixes #15505

Summary:

• LiftCoverage extended LiftComplex, it now extends LiftImpure and forces the lifting of non-erased applications
• StringContext.s, f, raw are now excluded from lifting
• ElimRepeated didn't handle lifted arguments properly, because it assumed that every argument was a Typed tree, which is not the case for arguments lifted by the coverage transformation. This is now fixed.

Most of the added lines are tests :)

How I fixed it

Let's see if I can imitate the awesome explanations of Martin 😄

InstrumentCoverage modifies the AST just after the pickling, and insert calls to scala.runtime.coverage.Invoker.invoked that record, at runtime, which methods are called.
See an example here. To lift functions' arguments, it uses LiftCoverage, which lives with its LiftXX siblings in EtaExpansion.scala.

LiftCoverage previously extended LiftComplex. A quick look at the definition of LiftComplex tells me that it lifts everything, except trees that are constants or have the maximal purity PurePath. And TreeInfo.exprPurity says that erased things are Pure, not PurePath! It seems that LiftImpure would be more appropriate than LiftComplex. Therefore, I happily type LiftCoverage extends LiftImpure. But stopping there would introduce another bug!

For example, given this code:

def f() = throw RuntimeException() // throws an exception
def g() = 0 // pure function
someFunction(f(), g())

We want to generate something like this, even if g has no side effects:

Invoker.invoked("f", ...)
val _f = f()
Invoker.invoked("g", ...)
val _g = g()
Invoker.invoked("someFunction", ...)
someFunction(_f, _g)

This way, f will throw its exception and prevent g from being marked as covered, which would be wrong.

Therefore, all non-erased applications must be lifted. This means that, when lifting arguments, LiftCoverage.noLift must return false for all non-erased Apply:

override def noLift(expr: tpd.Tree)(using Context) =
  if liftingArgs then noLiftArg(expr) else super.noLift(expr)

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
    // the other cases recurse on more complex trees that may contain applications

Similarly, string interpolators s, f and raw shouldn't be lifted. I've excluded those three functions from lifting in InstrumentCoverage.needsLift.

Finally, I took a look at the mysterious issue #15078.
For a call to this java method:

Path createTempDirectory(String prefix, FileAttribute<?>... attrs)

-Ycheck-all reported the following problem:

java.lang.AssertionError: assertion failed: Found:    ($2$ : java.nio.file.attribute.FileAttribute[?]*)
Required: Array[? <: java.nio.file.attribute.FileAttribute[?]]

And the code crashed at runtime...

Yet again, it only happened when the arguments were lifted by -coverage-out. A quick minimization revealed that the problem was the handling of the java varargs, in the presence of lifted arguments.

I know that ElimRepeated is the phase that takes care of handling varargs. Some debugging showed that ElimRepeated.transformApply was triggered, with:

• isWildcardStarArg(arg): true
• arg of type Ident

isWildcardStarArg properly handled Ident, but not ElimRepeated, which assumed that everything was a Typed tree. Fixing that made the bug vanish.

[smarter]

Otherwise LGTM!

[smarter]

Looks like the ci is failing with a coverage test check file mismatch

[TheElectronWill]

Ah yes, it must be because of #15504. I'll update the checkfiles

[smarter]

LGTM! If you have the time, I think this is something that would be worth backporting to the release-3.2.0 branch too (and #15504 too presumably?)

[TheElectronWill]

LGTM! If you have the time, I think this is something that would be worth backporting to the release-3.2.0 branch too (and #15504 too presumably?)

Sure! Here you are: #15573 :)