Scoverage: fix broken warnings related to tail recursion and infinite loop detection (#25941)

Fixes a bunch of warnings broken by scoverage related to tail recursion
and infinite loops detection. Warnings were broken by:

- Coverage modifying the trees to inject instrumentation, breaking
assumptions by the loop detection and tail recursion phases.
- Coverage modifying the spans of the trees when instrumenting them,
which breaks the positions at which warnings are reported.

The solution by phases affected:

**`InstrumentCoverage`** - phase that instruments the trees to inject
coverage probes:
- Do not instrument literal booleans in `if` conditions - they are
getting in the way of rewriting `if` statements with literal conditions
into their respective `then` or `else` branches.
- Preserve the spans of the trees when instrumenting them, so that
warnings are reported at the original position of the code.

**`CheckLoopingImplicits`** - phase that detects infinite loops in
initializers:
- Ignore coverage probes as ones not capable of leading to an infinite
loop.
- Strip the coverage instrumentation from RHS of defs before analyzing
them for loops.

**`TailRec`** - phase that rewrites tail recursive calls into loops:
- Modified `isInfiniteRecCall` - the method that detects infinite loops
in tail recursive calls - to allow coverage probes as ones not capable
of breaking an infinite loop.
- In `transform`, modify the default getter index in recursive call
argument position to peel through the coverage instrumentation to
recover the original default-getter parameter index.

## How much have you relied on LLM-based tools in this contribution?

Moderately, for codebase analysis and tracing.

## How was the solution tested?

`sbt "testCompilation --enable-coverage-phase"`

Author: anatoliykmetyuk

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

@@ -54,6 +54,8 @@ class CheckLoopingImplicits extends MiniPhase:
             )
 
     def checkNotLooping(t: Tree): Unit = t match
+      case t if InstrumentCoverage.isCoverageProbe(t) =>
+        ()
       case t: Ident =>
         checkNotSelfRef(t)
       case t @ Select(qual, _) =>
@@ -105,7 +107,7 @@ class CheckLoopingImplicits extends MiniPhase:
     if sym.isOneOf(GivenOrImplicit | Lazy | ExtensionMethod)
       || sym.name == nme.apply && sym.owner.is(Module) && sym.owner.sourceModule.isOneOf(GivenOrImplicit)
     then
-      checkNotLooping(mdef.rhs)
+      checkNotLooping(InstrumentCoverage.stripLeadingCoverage(mdef.rhs))
     mdef
   end transform
 end CheckLoopingImplicits

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

@@ -341,7 +341,11 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
         val allProbes = inheritedProbes :+ coverageCall
         allProbes match
           case single :: Nil => InstrumentedParts.singleExprTree(single, transformed)
-          case multiple => Block(multiple, transformed)
+          case multiple => InstrumentCoverage.blockWithExprSpan(multiple, transformed)
+
+    private def transformCondition(tree: Tree)(using Context): Tree = tree match
+      case Literal(Constant(_: Boolean)) => tree
+      case _ => transform(tree)
 
     override def transform(tree: Tree)(using Context): Tree =
       inContext(transformCtx(tree)) { // necessary to position inlined code properly
@@ -366,7 +370,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
           // branches
           case tree: If =>
             cpy.If(tree)(
-              cond = transform(tree.cond),
+              cond = transformCondition(tree.cond),
               thenp = transformBranch(tree.thenp),
               elsep = transformBranch(tree.elsep)
             )
@@ -400,7 +404,7 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
               // This is especially important for trees like (expr[T])(args),
               // for which the wrong transformation crashes the compiler.
               // See tests/coverage/pos/PolymorphicExtensions.scala
-              Block(
+              InstrumentCoverage.blockWithExprSpan(
                 pre :+ coverageCall,
                 cpy.TypeApply(tree)(expr, args)
               )
@@ -750,6 +754,33 @@ object InstrumentCoverage:
   val scoverageLocalOn: Regex = """^\s*//\s*\$COVERAGE-ON\$""".r
   val scoverageLocalOff: Regex = """^\s*//\s*\$COVERAGE-OFF\$""".r
 
+  /** Coverage probes are synthetic bookkeeping calls that should be transparent to
+   *  later warning logic and should not steal source positions from the user tree
+   *  they wrap.
+   */
+  def isCoverageProbe(tree: Tree)(using Context): Boolean = tree match
+    case Apply(fun, Literal(Constant(_: Int)) :: Literal(Constant(_: String)) :: Nil) =>
+      fun.symbol == defn.InvokedMethodRef.symbol
+    case _ =>
+      false
+
+  /** Remove leading synthetic coverage wrappers to recover the user-written tree. */
+  def stripLeadingCoverage(tree: Tree)(using Context): Tree = tree match
+    case Typed(expr, _) =>
+      stripLeadingCoverage(expr)
+    case Inlined(_, Nil, expr) =>
+      stripLeadingCoverage(expr)
+    case Block(stats, expr) if stats.forall(isCoverageProbe) =>
+      stripLeadingCoverage(expr)
+    case _ =>
+      tree
+
+  /** Keep wrapper blocks pointed at the wrapped expression span so later warnings
+   *  still highlight user code instead of synthetic `Invoker.invoked` scaffolding.
+   */
+  def blockWithExprSpan(stats: List[Tree], expr: Tree)(using Context): Tree =
+    Block(stats, expr).withSpan(expr.span)
+
   /**
    * An instrumented Tree, in 3 parts.
    * @param pre preparation code, e.g. lifted arguments. May be empty.
@@ -762,13 +793,13 @@ object InstrumentCoverage:
     /** Turns this into an actual Tree. */
     def toTree(using Context): Tree =
       if invokeCall.isEmpty then expr
-      else if pre.isEmpty then Block(invokeCall :: Nil, expr)
-      else Block(pre :+ invokeCall, expr)
+      else if pre.isEmpty then blockWithExprSpan(invokeCall :: Nil, expr)
+      else blockWithExprSpan(pre :+ invokeCall, expr)
 
   object InstrumentedParts:
     def notCovered(expr: Tree) = InstrumentedParts(Nil, EmptyTree, expr)
     def singleExpr(invokeCall: Apply, expr: Tree) = InstrumentedParts(Nil, invokeCall, expr)
 
     /** Shortcut for `singleExpr(call, expr).toTree` */
     def singleExprTree(invokeCall: Apply, expr: Tree)(using Context): Tree =
-      Block(invokeCall :: Nil, expr)
+      blockWithExprSpan(invokeCall :: Nil, expr)

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

@@ -195,6 +195,7 @@ class TailRec extends MiniPhase {
          */
         def isInfiniteRecCall(tree: Tree): Boolean = {
           def tailArgOrPureExpr(stat: Tree): Boolean = stat match {
+            case stat if InstrumentCoverage.isCoverageProbe(stat) => true
             case stat: ValDef if stat.name.is(TailTempName) || !stat.symbol.is(Mutable) => tailArgOrPureExpr(stat.rhs)
             case Assign(lhs: Ident, rhs) if lhs.symbol.name.is(TailLocalName) =>
               tailArgOrPureExpr(rhs) || varForRewrittenThis.exists(_ == lhs.symbol && rhs.tpe.isStable)
@@ -325,11 +326,31 @@ class TailRec extends MiniPhase {
           method.matches(calledMethod) &&
           enclosingClass.appliedRef.widen <:< prefix.tpe.widenDealias
 
+        // Argument shape under coverage: `{ Invoker.invoked(...); f$default$n(...) }`;
+        // strip the probe block and recover `f$default$n`'s parameter index.
+        def defaultGetterIndex(arg: Tree): Option[Int] =
+          def fromSymbol(sym: Symbol): Option[Int] =
+            if sym.exists && sym.name.is(DefaultGetterName) then
+              val DefaultGetterName(_, index) = sym.name: @unchecked
+              Some(index)
+            else
+              None
+
+          val stripped = InstrumentCoverage.stripLeadingCoverage(arg)
+          fromSymbol(stripped.symbol).orElse {
+            stripped match
+              case id: Ident =>
+                id.symbol.defTree match
+                  case vdef: ValDef => defaultGetterIndex(vdef.rhs)
+                  case _ => None
+              case _ =>
+                None
+          }
+
         if isRecursiveCall then
           if ctx.settings.Whas.recurseWithDefault then
-            tree.args.find(_.symbol.name.is(DefaultGetterName)) match
-            case Some(arg) =>
-              val DefaultGetterName(_, index) = arg.symbol.name: @unchecked
+            tree.args.iterator.flatMap(defaultGetterIndex).nextOption() match
+            case Some(index) =>
               report.warning(RecurseWithDefault(calledMethod.info.firstParamNames(index)), tree.srcPos)
             case _ =>
 

compiler/test/dotc/scoverage-ignore.excludelist

@@ -15,32 +15,25 @@ i10889.scala
 i11247.scala
 i11556.scala
 i12739.scala
-i13011.scala
-i13542.scala
 i14164.scala
 i14947.scala
 i15165.scala
 i15864.scala
 i18263.orig.scala
 i18263.scala
 i18589
-i19505.scala
 i19955a.scala
 i19955b.scala
 i20053b.scala
 i21313.scala
 i2146.scala
 i23179.scala
-i23277.scala
 i23489.scala
-i23541.scala
-i23693.scala
 i24039.scala
 i5039.scala
 i8623.scala
 i8900a3.scala
 i9228.scala
-i9880.scala
 minicheck.scala
 minicheck-toplevel.scala
 mt-scrutinee-widen3.scala

tests/coverage/pos/SimpleMethods.scoverage.check

@@ -229,23 +229,6 @@ C
 Class
 covtest.C
 cond
-195
-200
-15
-<none>
-Literal
-false
-0
-false
-false
-
-13
-SimpleMethods.scala
-covtest
-C
-Class
-covtest.C
-cond
 206
 210
 15
@@ -256,7 +239,7 @@ false
 false
 true
 
-14
+13
 SimpleMethods.scala
 covtest
 C
@@ -273,7 +256,7 @@ true
 false
 true
 
-15
+14
 SimpleMethods.scala
 covtest
 C
@@ -290,7 +273,7 @@ false
 false
 false
 
-16
+15
 SimpleMethods.scala
 covtest
 C
@@ -307,7 +290,7 @@ true
 false
 false
 
-17
+16
 SimpleMethods.scala
 covtest
 C
@@ -324,24 +307,7 @@ false
 false
 def cond
 
-18
-SimpleMethods.scala
-covtest
-C
-Class
-covtest.C
-partialCond
-260
-265
-19
-<none>
-Literal
-false
-0
-false
-false
-
-19
+17
 SimpleMethods.scala
 covtest
 C
@@ -358,7 +324,7 @@ false
 false
 ()
 
-20
+18
 SimpleMethods.scala
 covtest
 C
@@ -375,7 +341,7 @@ true
 false
 ()
 
-21
+19
 SimpleMethods.scala
 covtest
 C
@@ -392,7 +358,7 @@ true
 false
 
 
-22
+20
 SimpleMethods.scala
 covtest
 C
@@ -409,7 +375,7 @@ false
 false
 def partialCond
 
-23
+21
 SimpleMethods.scala
 covtest
 C
@@ -426,7 +392,7 @@ false
 false
 new {}
 
-24
+22
 SimpleMethods.scala
 covtest
 C
@@ -443,7 +409,7 @@ false
 false
 def new1
 
-25
+23
 SimpleMethods.scala
 covtest
 C
@@ -460,7 +426,7 @@ false
 false
 ()
 
-26
+24
 SimpleMethods.scala
 covtest
 C
@@ -477,7 +443,7 @@ true
 false
 ()
 
-27
+25
 SimpleMethods.scala
 covtest
 C
@@ -494,7 +460,7 @@ false
 false
 1
 
-28
+26
 SimpleMethods.scala
 covtest
 C
@@ -511,7 +477,7 @@ true
 false
 => 1
 
-29
+27
 SimpleMethods.scala
 covtest
 C