Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a second trace for global init checker showing creation of mutable fields #19996

Merged
merged 3 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
45 changes: 29 additions & 16 deletions compiler/src/dotty/tools/dotc/transform/init/Objects.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 + ")"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 _ =>
Expand Down Expand Up @@ -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 _ =>
Expand Down Expand Up @@ -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)
10 changes: 10 additions & 0 deletions tests/init-global/warn/TypeCast.check
Original file line number Diff line number Diff line change
@@ -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 ]
| ^
18 changes: 18 additions & 0 deletions tests/init-global/warn/TypeCast.scala
Original file line number Diff line number Diff line change
@@ -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
}

5 changes: 5 additions & 0 deletions tests/init-global/warn/global-irrelevance5.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^^^^^^^^^^^^
5 changes: 5 additions & 0 deletions tests/init-global/warn/global-irrelevance6.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^^^^^^^^^^^^
5 changes: 5 additions & 0 deletions tests/init-global/warn/global-irrelevance7.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^^^^^^^^^^^^
7 changes: 7 additions & 0 deletions tests/init-global/warn/mutable-array.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
7 changes: 7 additions & 0 deletions tests/init-global/warn/mutable-read1.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
7 changes: 7 additions & 0 deletions tests/init-global/warn/mutable-read2.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^
7 changes: 7 additions & 0 deletions tests/init-global/warn/mutable-read3.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
7 changes: 7 additions & 0 deletions tests/init-global/warn/mutable-read4.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
7 changes: 7 additions & 0 deletions tests/init-global/warn/mutable-read6.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^
5 changes: 5 additions & 0 deletions tests/init-global/warn/patmat-unapplySeq.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^^^^^^^^^^^^
5 changes: 5 additions & 0 deletions tests/init-global/warn/patmat-unapplySeq2.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
| ^^^^^^^^^^^^
Loading