Fix #17042: Preserve the shape of secondary ctors in instrumentCoverage. (#17111)

Author: sjrd

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

@@ -11,6 +11,7 @@ import core.DenotTransformers.IdentityDenotTransformer
 import core.Symbols.{defn, Symbol}
 import core.Constants.Constant
 import core.NameOps.isContextFunction
+import core.StdNames.nme
 import core.Types.*
 import coverage.*
 import typer.LiftCoverage
@@ -325,7 +326,11 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
         // Only transform the params (for the default values) and the rhs, not the name and tpt.
         val transformedParamss = transformParamss(tree.paramss)
         val transformedRhs =
-          if !sym.isOneOf(Accessor | Artifact | Synthetic) && !tree.rhs.isEmpty then
+          if tree.rhs.isEmpty then
+            tree.rhs
+          else if sym.isClassConstructor then
+            instrumentSecondaryCtor(tree)
+          else if !sym.isOneOf(Accessor | Artifact | Synthetic) then
             // If the body can be instrumented, do it (i.e. insert a "coverage call" at the beginning)
             // This is useful because methods can be stored and called later, or called by reflection,
             // and if the rhs is too simple to be instrumented (like `def f = this`),
@@ -410,6 +415,24 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
           val coverageCall = createInvokeCall(parent, pos)
           InstrumentedParts.singleExprTree(coverageCall, body)
 
+    /** Instruments the body of a secondary constructor DefDef.
+     *
+     *  We must preserve the delegate constructor call as the first statement of
+     *  the rhs Block, otherwise `HoistSuperArgs` will not be happy (see #17042).
+     */
+    private def instrumentSecondaryCtor(ctorDef: DefDef)(using Context): Tree =
+      // compute position like in instrumentBody
+      val namePos = ctorDef.namePos
+      val pos = namePos.withSpan(namePos.span.withStart(ctorDef.span.start))
+      val coverageCall = createInvokeCall(ctorDef, pos)
+
+      ctorDef.rhs match
+        case b @ Block(delegateCtorCall :: stats, expr: Literal) =>
+          cpy.Block(b)(transform(delegateCtorCall) :: coverageCall :: stats.mapConserve(transform), expr)
+        case rhs =>
+          cpy.Block(rhs)(transform(rhs) :: coverageCall :: Nil, unitLiteral)
+    end instrumentSecondaryCtor
+
     /**
      * Checks if the apply needs a lift in the coverage phase.
      * In case of a nested application, we have to lift all arguments
@@ -447,9 +470,14 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
 
     /** Check if an Apply can be instrumented. Prevents this phase from generating incorrect code. */
     private def canInstrumentApply(tree: Apply)(using Context): Boolean =
+      def isSecondaryCtorDelegateCall: Boolean = tree.fun match
+        case Select(This(_), nme.CONSTRUCTOR) => true
+        case _                                => false
+
       val sym = tree.symbol
       !sym.isOneOf(ExcludeMethodFlags)
       && !isCompilerIntrinsicMethod(sym)
+      && !(sym.isClassConstructor && isSecondaryCtorDelegateCall)
       && (tree.typeOpt match
         case AppliedType(tycon: NamedType, _) =>
           /* If the last expression in a block is a context function, we'll try to

tests/coverage/pos/Constructor.scala

@@ -1,10 +1,20 @@
 package covtest
 
 class C:
+  def this(arg: String) = {
+    this()
+    g()
+  }
+
+  def this(x: Int) =
+    this(x.toString() + "foo")
+
   def f(x: Int) = x
   def x = 1
   f(x)
 
+  def g(): Int = 2
+
 object O:
   def g(y: Int) = y
   def y = 1

tests/coverage/pos/Constructor.scoverage.check

@@ -24,129 +24,214 @@ covtest
 C
 Class
 covtest.C
-f
+<init>
 28
-33
+36
 3
+<init>
+DefDef
+false
+0
+false
+def this
+
+1
+Constructor.scala
+covtest
+C
+Class
+covtest.C
+<init>
+69
+72
+5
+g
+Apply
+false
+0
+false
+g()
+
+2
+Constructor.scala
+covtest
+C
+Class
+covtest.C
+<init>
+80
+88
+8
+<init>
+DefDef
+false
+0
+false
+def this
+
+3
+Constructor.scala
+covtest
+C
+Class
+covtest.C
+<init>
+108
+128
+9
++
+Apply
+false
+0
+false
+x.toString() + "foo"
+
+4
+Constructor.scala
+covtest
+C
+Class
+covtest.C
+f
+133
+138
+11
 f
 DefDef
 false
 0
 false
 def f
 
-1
+5
 Constructor.scala
 covtest
 C
 Class
 covtest.C
 x
-48
-53
-4
+153
+158
+12
 x
 DefDef
 false
 0
 false
 def x
 
-2
+6
 Constructor.scala
 covtest
 C
 Class
 covtest.C
 <init>
-60
-64
-5
+165
+169
+13
 f
 Apply
 false
 0
 false
 f(x)
 
-3
+7
 Constructor.scala
 covtest
 C
 Class
 covtest.C
 <init>
-62
-63
-5
+167
+168
+13
 x
 Select
 false
 0
 false
 x
 
-4
+8
+Constructor.scala
+covtest
+C
+Class
+covtest.C
+g
+173
+178
+15
+g
+DefDef
+false
+0
+false
+def g
+
+9
 Constructor.scala
 covtest
 O$
 Object
 covtest.O$
 g
-78
-83
-8
+203
+208
+18
 g
 DefDef
 false
 0
 false
 def g
 
-5
+10
 Constructor.scala
 covtest
 O$
 Object
 covtest.O$
 y
-98
-103
-9
+223
+228
+19
 y
 DefDef
 false
 0
 false
 def y
 
-6
+11
 Constructor.scala
 covtest
 O$
 Object
 covtest.O$
 <init>
-110
-114
-10
+235
+239
+20
 g
 Apply
 false
 0
 false
 g(y)
 
-7
+12
 Constructor.scala
 covtest
 O$
 Object
 covtest.O$
 <init>
-112
-113
-10
+237
+238
+20
 y
 Ident
 false