Added a second trace for global init checker showing creation of muta…

…ble fields (#19996)
scala · Mar 25, 2024 · a5bb6be · a5bb6be
2 parents 18b146b + fd6aaf7
commit a5bb6be
Show file tree

Hide file tree

Showing 15 changed files with 126 additions and 16 deletions.
diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala
@@ -167,6 +167,8 @@ class Compiler {
     val rctx =
       if ctx.settings.Xsemanticdb.value then
         ctx.addMode(Mode.ReadPositions)
+      else if ctx.settings.YcheckInitGlobal.value then
+        ctx.addMode(Mode.ReadPositions)
       else
         ctx
     new Run(this, rctx)

diff --git a/compiler/src/dotty/tools/dotc/transform/init/Objects.scala b/compiler/src/dotty/tools/dotc/transform/init/Objects.scala
@@ -197,7 +197,7 @@ class Objects:
    *
    * @param owner The static object whose initialization creates the array.
    */
-  case class OfArray(owner: ClassSymbol, regions: Regions.Data)(using @constructorOnly ctx: Context) extends ValueElement:
+  case class OfArray(owner: ClassSymbol, regions: Regions.Data)(using @constructorOnly ctx: Context, @constructorOnly trace: Trace) extends ValueElement:
     val klass: ClassSymbol = defn.ArrayClass
     val addr: Heap.Addr = Heap.arrayAddr(regions, owner)
     def show(using Context) = "OfArray(owner = " + owner.show + ")"
@@ -455,9 +455,11 @@ class Objects:
     abstract class Addr:
       /** The static object which owns the mutable slot */
       def owner: ClassSymbol
+      def getTrace: Trace = Trace.empty
 
     /** The address for mutable fields of objects. */
-    private case class FieldAddr(regions: Regions.Data, field: Symbol, owner: ClassSymbol) extends Addr
+    private case class FieldAddr(regions: Regions.Data, field: Symbol, owner: ClassSymbol)(trace: Trace) extends Addr:
+      override def getTrace: Trace = trace
 
     /** The address for mutable local variables . */
     private case class LocalVarAddr(regions: Regions.Data, sym: Symbol, owner: ClassSymbol) extends Addr
@@ -497,11 +499,11 @@ class Objects:
     def localVarAddr(regions: Regions.Data, sym: Symbol, owner: ClassSymbol): Addr =
       LocalVarAddr(regions, sym, owner)
 
-    def fieldVarAddr(regions: Regions.Data, sym: Symbol, owner: ClassSymbol): Addr =
-      FieldAddr(regions, sym, owner)
+    def fieldVarAddr(regions: Regions.Data, sym: Symbol, owner: ClassSymbol)(using Trace): Addr =
+      FieldAddr(regions, sym, owner)(summon[Trace])
 
-    def arrayAddr(regions: Regions.Data, owner: ClassSymbol)(using Context): Addr =
-      FieldAddr(regions, defn.ArrayClass, owner)
+    def arrayAddr(regions: Regions.Data, owner: ClassSymbol)(using Trace, Context): Addr =
+      FieldAddr(regions, defn.ArrayClass, owner)(summon[Trace])
 
     def getHeapData()(using mutable: MutableData): Data = mutable.heap
 
@@ -654,12 +656,12 @@ class Objects:
         if arr.addr.owner == State.currentObject then
           Heap.read(arr.addr)
         else
-          errorReadOtherStaticObject(State.currentObject, arr.addr.owner)
+          errorReadOtherStaticObject(State.currentObject, arr.addr)
           Bottom
       else if target == defn.Array_update then
         assert(args.size == 2, "Incorrect number of arguments for Array update, found = " + args.size)
         if arr.addr.owner != State.currentObject then
-          errorMutateOtherStaticObject(State.currentObject, arr.addr.owner)
+          errorMutateOtherStaticObject(State.currentObject, arr.addr)
         else
           Heap.writeJoin(arr.addr, args.tail.head.value)
         Bottom
@@ -810,7 +812,7 @@ class Objects:
             if addr.owner == State.currentObject then
               Heap.read(addr)
             else
-              errorReadOtherStaticObject(State.currentObject, addr.owner)
+              errorReadOtherStaticObject(State.currentObject, addr)
               Bottom
           else if ref.isObjectRef && ref.klass.hasSource then
             report.warning("Access uninitialized field " + field.show + ". " + Trace.show, Trace.position)
@@ -879,7 +881,7 @@ class Objects:
       if ref.hasVar(field) then
         val addr = ref.varAddr(field)
         if addr.owner != State.currentObject then
-          errorMutateOtherStaticObject(State.currentObject, addr.owner)
+          errorMutateOtherStaticObject(State.currentObject, addr)
         else
           Heap.writeJoin(addr, rhs)
       else
@@ -968,7 +970,7 @@ class Objects:
           if addr.owner == State.currentObject then
             Heap.read(addr)
           else
-            errorReadOtherStaticObject(State.currentObject, addr.owner)
+            errorReadOtherStaticObject(State.currentObject, addr)
             Bottom
           end if
         case _ =>
@@ -1020,7 +1022,7 @@ class Objects:
       Env.getVar(sym) match
       case Some(addr) =>
         if addr.owner != State.currentObject then
-          errorMutateOtherStaticObject(State.currentObject, addr.owner)
+          errorMutateOtherStaticObject(State.currentObject, addr)
         else
           Heap.writeJoin(addr, value)
       case _ =>
@@ -1757,20 +1759,31 @@ class Objects:
       if cls.isAllOf(Flags.JavaInterface) then Bottom
       else evalType(tref.prefix, thisV, klass, elideObjectAccess = cls.isStatic)
 
+  def printTraceWhenMultiple(trace: Trace)(using Context): String =
+    if trace.toVector.size > 1 then
+      Trace.buildStacktrace(trace, "The mutable state is created through: " + System.lineSeparator())
+    else ""
+
   val mutateErrorSet: mutable.Set[(ClassSymbol, ClassSymbol)] = mutable.Set.empty
-  def errorMutateOtherStaticObject(currentObj: ClassSymbol, otherObj: ClassSymbol)(using Trace, Context) =
+  def errorMutateOtherStaticObject(currentObj: ClassSymbol, addr: Heap.Addr)(using Trace, Context) =
+    val otherObj = addr.owner
+    val addr_trace = addr.getTrace
     if mutateErrorSet.add((currentObj, otherObj)) then
       val msg =
         s"Mutating ${otherObj.show} during initialization of ${currentObj.show}.\n" +
-        "Mutating other static objects during the initialization of one static object is forbidden. " + Trace.show
+        "Mutating other static objects during the initialization of one static object is forbidden. " + Trace.show +
+        printTraceWhenMultiple(addr_trace)
 
       report.warning(msg, Trace.position)
 
   val readErrorSet: mutable.Set[(ClassSymbol, ClassSymbol)] = mutable.Set.empty
-  def errorReadOtherStaticObject(currentObj: ClassSymbol, otherObj: ClassSymbol)(using Trace, Context) =
+  def errorReadOtherStaticObject(currentObj: ClassSymbol, addr: Heap.Addr)(using Trace, Context) =
+    val otherObj = addr.owner
+    val addr_trace = addr.getTrace
     if readErrorSet.add((currentObj, otherObj)) then
       val msg =
         "Reading mutable state of " + otherObj.show + " during initialization of " + currentObj.show + ".\n" +
-        "Reading mutable state of other static objects is forbidden as it breaks initialization-time irrelevance. " + Trace.show
+        "Reading mutable state of other static objects is forbidden as it breaks initialization-time irrelevance. " + Trace.show +
+        printTraceWhenMultiple(addr_trace)
 
       report.warning(msg, Trace.position)
diff --git a/tests/init-global/warn/TypeCast.check b/tests/init-global/warn/TypeCast.check
@@ -0,0 +1,10 @@
+-- Warning: tests/init-global/warn/TypeCast.scala:7:17 -----------------------------------------------------------------
+7 |  def g(): Int = f // warn
+  |                 ^
+  |                 Access uninitialized field value f. Calling trace:
+  |                 ├── object B {	[ TypeCast.scala:5 ]
+  |                 │   ^
+  |                 ├── val f: Int = g()	[ TypeCast.scala:6 ]
+  |                 │                ^^^
+  |                 └── def g(): Int = f // warn	[ TypeCast.scala:7 ]
+  |                                    ^
diff --git a/tests/init-global/warn/TypeCast.scala b/tests/init-global/warn/TypeCast.scala
@@ -0,0 +1,18 @@
+object A {
+  val f: Int = 10
+  def m() = f
+}
+object B {
+  val f: Int = g()
+  def g(): Int = f // warn
+}
+object C {
+  val a: A.type | B.type = if ??? then A else B
+  def cast[T](a: Any): T = a.asInstanceOf[T]
+  val c: A.type = cast[A.type](a) // abstraction for c is {A, B}
+  val d = c.f // treat as c.asInstanceOf[owner of f].f
+  val e = c.m() // treat as c.asInstanceOf[owner of f].m()
+  val c2: B.type = cast[B.type](a)
+  val g = c2.f // no error here
+}
+
diff --git a/tests/init-global/warn/global-irrelevance5.check b/tests/init-global/warn/global-irrelevance5.check
@@ -7,3 +7,8 @@
   |│   ^
   |└── var y = A.array(0) * 2 // warn	[ global-irrelevance5.scala:6 ]
   |            ^^^^^^^^^^
+  |The mutable state is created through: 
+  |├── object A:	[ global-irrelevance5.scala:1 ]
+  |│   ^
+  |└── val array: Array[Int] = new Array(1)	[ global-irrelevance5.scala:2 ]
+  |                            ^^^^^^^^^^^^
diff --git a/tests/init-global/warn/global-irrelevance6.check b/tests/init-global/warn/global-irrelevance6.check
@@ -7,3 +7,8 @@
   |│   ^
   |└── var y = A.array(0).foo() * 2 // warn	[ global-irrelevance6.scala:9 ]
   |            ^^^^^^^^^^
+  |The mutable state is created through: 
+  |├── object A:	[ global-irrelevance6.scala:4 ]
+  |│   ^
+  |└── val array: Array[Box] = new Array(1)	[ global-irrelevance6.scala:5 ]
+  |                            ^^^^^^^^^^^^
diff --git a/tests/init-global/warn/global-irrelevance7.check b/tests/init-global/warn/global-irrelevance7.check
@@ -7,3 +7,8 @@
    |│   ^
    |└── var y = A.array(0).foo() * 2 // warn	[ global-irrelevance7.scala:10 ]
    |            ^^^^^^^^^^
+   |The mutable state is created through: 
+   |├── object A:	[ global-irrelevance7.scala:4 ]
+   |│   ^
+   |└── val array: Array[Box] = new Array(1)	[ global-irrelevance7.scala:5 ]
+   |                            ^^^^^^^^^^^^
diff --git a/tests/init-global/warn/mutable-array.check b/tests/init-global/warn/mutable-array.check
@@ -7,3 +7,10 @@
   |│   ^
   |└── val x: Int = box.value // warn	[ mutable-array.scala:8 ]
   |                 ^^^^^^^^^
+  |The mutable state is created through: 
+  |├── object A:	[ mutable-array.scala:1 ]
+  |│   ^
+  |├── val box: Box = new Box(0)	[ mutable-array.scala:3 ]
+  |│                  ^^^^^^^^^^
+  |└── class Box(var value: Int)	[ mutable-array.scala:2 ]
+  |    ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/init-global/warn/mutable-read1.check b/tests/init-global/warn/mutable-read1.check
@@ -7,3 +7,10 @@
    |│   ^
    |└── val n: Int = boxA.value // warn	[ mutable-read1.scala:10 ]
    |                 ^^^^^^^^^^
+   |The mutable state is created through: 
+   |├── object A:	[ mutable-read1.scala:3 ]
+   |│   ^
+   |├── val box: Box = new Box(4)	[ mutable-read1.scala:4 ]
+   |│                  ^^^^^^^^^^
+   |└── class Box(var value: Int)	[ mutable-read1.scala:1 ]
+   |    ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/init-global/warn/mutable-read2.check b/tests/init-global/warn/mutable-read2.check
@@ -7,3 +7,10 @@
    |│   ^
    |└── val b: Int = box.value // warn	[ mutable-read2.scala:10 ]
    |                 ^^^^^^^^^
+   |The mutable state is created through: 
+   |├── object A:	[ mutable-read2.scala:1 ]
+   |│   ^
+   |├── val box: Box = new Box(0)	[ mutable-read2.scala:5 ]
+   |│                  ^^^^^^^^^^
+   |└── class Box(var value: Int) {	[ mutable-read2.scala:2 ]
+   |    ^
diff --git a/tests/init-global/warn/mutable-read3.check b/tests/init-global/warn/mutable-read3.check
@@ -7,3 +7,10 @@
   |│   ^
   |└── val x: Int = box.value // warn	[ mutable-read3.scala:9 ]
   |                 ^^^^^^^^^
+  |The mutable state is created through: 
+  |├── object A:	[ mutable-read3.scala:1 ]
+  |│   ^
+  |├── val box: Box = new Box(0)	[ mutable-read3.scala:3 ]
+  |│                  ^^^^^^^^^^
+  |└── class Box(var value: Int)	[ mutable-read3.scala:2 ]
+  |    ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/init-global/warn/mutable-read4.check b/tests/init-global/warn/mutable-read4.check
@@ -7,3 +7,10 @@
    |│   ^
    |└── val n: Int = boxA.value // warn	[ mutable-read4.scala:10 ]
    |                 ^^^^^^^^^^
+   |The mutable state is created through: 
+   |├── object A:	[ mutable-read4.scala:3 ]
+   |│   ^
+   |├── val box: Box = new Box(4)	[ mutable-read4.scala:4 ]
+   |│                  ^^^^^^^^^^
+   |└── class Box(var value: Int)	[ mutable-read4.scala:1 ]
+   |    ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/init-global/warn/mutable-read6.check b/tests/init-global/warn/mutable-read6.check
@@ -9,3 +9,10 @@
   |│                                                           ^^^^^^^^^^^^^^^^
   |└── final def source: SourceFile = _source // warn	[ mutable-read6.scala:7 ]
   |                                   ^^^^^^^
+  |The mutable state is created through: 
+  |├── object Contexts:	[ mutable-read6.scala:3 ]
+  |│   ^
+  |├── val NoContext: Context = new Context	[ mutable-read6.scala:4 ]
+  |│                            ^^^^^^^^^^^
+  |└── class Context:	[ mutable-read6.scala:5 ]
+  |    ^
diff --git a/tests/init-global/warn/patmat-unapplySeq.check b/tests/init-global/warn/patmat-unapplySeq.check
@@ -9,3 +9,8 @@
   |│        ^^^^
   |└── def apply(i: Int): Box = array(i) // warn	[ patmat-unapplySeq.scala:8 ]
   |                             ^^^^^^^^
+  |The mutable state is created through: 
+  |├── object A:	[ patmat-unapplySeq.scala:1 ]
+  |│   ^
+  |└── val array: Array[Box] = new Array(1)	[ patmat-unapplySeq.scala:4 ]
+  |                            ^^^^^^^^^^^^
diff --git a/tests/init-global/warn/patmat-unapplySeq2.check b/tests/init-global/warn/patmat-unapplySeq2.check
@@ -9,3 +9,8 @@
   |│        ^^^^^
   |└── def apply(i: Int): Box = array(i) // warn	[ patmat-unapplySeq2.scala:8 ]
   |                             ^^^^^^^^
+  |The mutable state is created through: 
+  |├── object A:	[ patmat-unapplySeq2.scala:1 ]
+  |│   ^
+  |└── val array: Array[Box] = new Array(1)	[ patmat-unapplySeq2.scala:4 ]
+  |                            ^^^^^^^^^^^^