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.

Sign up for GitHub

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

Make sure that trace is shown correctly in the presence of invalid line numbers #18930

Merged
merged 2 commits into from
Nov 17, 2023
Merged

Make sure that trace is shown correctly in the presence of invalid line numbers #18930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dde0a0c
Improve trace for TASTy without valid line numbers
liufengyun Nov 15, 2023
395167a
Update check files
liufengyun Nov 15, 2023
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
30 changes: 14 additions & 16 deletions compiler/src/dotty/tools/dotc/transform/init/Objects.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ object Objects:
case Some(theValue) =>
theValue
case _ =>
report.warning("[Internal error] Value not found " + x.show + "\nenv = " + data.show + ". Calling trace:\n" + Trace.show, Trace.position)
report.warning("[Internal error] Value not found " + x.show + "\nenv = " + data.show + ". " + Trace.show, Trace.position)
Bottom

def getVal(x: Symbol)(using data: Data, ctx: Context): Option[Value] = data.getVal(x)
Expand Down Expand Up @@ -619,7 +619,7 @@ object Objects:
def call(value: Value, meth: Symbol, args: List[ArgInfo], receiver: Type, superType: Type, needResolve: Boolean = true): Contextual[Value] = log("call " + meth.show + ", this = " + value.show + ", args = " + args.map(_.value.show), printer, (_: Value).show) {
value match
case Cold =>
report.warning("Using cold alias. Calling trace:\n" + Trace.show, Trace.position)
report.warning("Using cold alias. " + Trace.show, Trace.position)
Bottom

case Bottom =>
Expand Down Expand Up @@ -781,15 +781,15 @@ object Objects:
errorReadOtherStaticObject(State.currentObject, addr.owner)
Bottom
else if ref.isObjectRef && ref.klass.hasSource then
report.warning("Access uninitialized field " + field.show + ". Call trace: " + Trace.show, Trace.position)
report.warning("Access uninitialized field " + field.show + ". " + Trace.show, Trace.position)
Bottom
else
// initialization error, reported by the initialization checker
Bottom
else if ref.hasVal(target) then
ref.valValue(target)
else if ref.isObjectRef && ref.klass.hasSource then
report.warning("Access uninitialized field " + field.show + ". Call trace: " + Trace.show, Trace.position)
report.warning("Access uninitialized field " + field.show + ". " + Trace.show, Trace.position)
Bottom
else
// initialization error, reported by the initialization checker
Expand Down Expand Up @@ -836,7 +836,7 @@ object Objects:
report.warning("[Internal error] unexpected tree in assignment, array = " + arr.show + Trace.show, Trace.position)

case Cold =>
report.warning("Assigning to cold aliases is forbidden. Calling trace:\n" + Trace.show, Trace.position)
report.warning("Assigning to cold aliases is forbidden. " + Trace.show, Trace.position)

case Bottom =>

Expand All @@ -851,7 +851,7 @@ object Objects:
else
Heap.writeJoin(addr, rhs)
else
report.warning("Mutating a field before its initialization: " + field.show + ". Calling trace:\n" + Trace.show, Trace.position)
report.warning("Mutating a field before its initialization: " + field.show + ". " + Trace.show, Trace.position)
end match

Bottom
Expand Down Expand Up @@ -936,7 +936,7 @@ object Objects:
Bottom
end if
case _ =>
report.warning("[Internal error] Variable not found " + sym.show + "\nenv = " + env.show + ". Calling trace:\n" + Trace.show, Trace.position)
report.warning("[Internal error] Variable not found " + sym.show + "\nenv = " + env.show + ". " + Trace.show, Trace.position)
Bottom
else
given Env.Data = env
Expand All @@ -948,17 +948,17 @@ object Objects:
given Env.Data = fun.env
eval(fun.code, fun.thisV, fun.klass)
case Cold =>
report.warning("Calling cold by-name alias. Call trace: \n" + Trace.show, Trace.position)
report.warning("Calling cold by-name alias. " + Trace.show, Trace.position)
Bottom
case _: ValueSet | _: Ref | _: OfArray =>
report.warning("[Internal error] Unexpected by-name value " + value.show + ". Calling trace:\n" + Trace.show, Trace.position)
report.warning("[Internal error] Unexpected by-name value " + value.show + ". " + Trace.show, Trace.position)
Bottom
else
value

case _ =>
if isByNameParam(sym) then
report.warning("Calling cold by-name alias. Call trace: \n" + Trace.show, Trace.position)
report.warning("Calling cold by-name alias. " + Trace.show, Trace.position)
Bottom
else
Cold
Expand All @@ -983,10 +983,10 @@ object Objects:
else
Heap.writeJoin(addr, value)
case _ =>
report.warning("[Internal error] Variable not found " + sym.show + "\nenv = " + env.show + ". Calling trace:\n" + Trace.show, Trace.position)
report.warning("[Internal error] Variable not found " + sym.show + "\nenv = " + env.show + ". " + Trace.show, Trace.position)

case _ =>
report.warning("Assigning to variables in outer scope. Calling trace:\n" + Trace.show, Trace.position)
report.warning("Assigning to variables in outer scope. " + Trace.show, Trace.position)

Bottom
}
Expand Down Expand Up @@ -1715,15 +1715,13 @@ object Objects:
def errorMutateOtherStaticObject(currentObj: ClassSymbol, otherObj: ClassSymbol)(using Trace, Context) =
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. " +
"Calling trace:\n" + Trace.show
"Mutating other static objects during the initialization of one static object is forbidden. " + Trace.show

report.warning(msg, Trace.position)

def errorReadOtherStaticObject(currentObj: ClassSymbol, otherObj: ClassSymbol)(using Trace, Context) =
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. " +
"Calling trace: " + Trace.show
"Reading mutable state of other static objects is forbidden as it breaks initialization-time irrelevance. " + Trace.show

report.warning(msg, Trace.position)
28 changes: 18 additions & 10 deletions compiler/src/dotty/tools/dotc/transform/init/Trace.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,17 @@ object Trace:

val empty: Trace = Vector.empty

val EMPTY_PADDING = " "
val CONNECTING_INDENT = "\u2502 " // "| "
val CHILD = "\u251c\u2500\u2500 " // "|-- "
val LAST_CHILD = "\u2514\u2500\u2500 " // "\-- "

extension (trace: Trace)
def add(node: Tree): Trace = trace :+ node
def toVector: Vector[Tree] = trace
def ++(trace2: Trace): Trace = trace ++ trace2

def show(using trace: Trace, ctx: Context): String = buildStacktrace(trace, "\n")
def show(using trace: Trace, ctx: Context): String = buildStacktrace(trace, "Calling trace:" + System.lineSeparator())

def position(using trace: Trace): Tree = trace.last

Expand All @@ -41,8 +46,8 @@ object Trace:
var lastLineNum = -1
var lines: mutable.ArrayBuffer[String] = new mutable.ArrayBuffer
trace.foreach { tree =>
val isLastTraceItem = tree `eq` trace.last
val pos = tree.sourcePos
val prefix = "-> "
val line =
if pos.source.exists then
val loc = "[ " + pos.source.file.name + ":" + (pos.line + 1) + " ]"
Expand All @@ -52,19 +57,22 @@ object Trace:
tree match
case defDef: DefTree =>
// The definition can be huge, avoid printing the whole definition.
defDef.symbol.show
defDef.symbol.showFullName
case _ =>
tree.show
tree.show.split(System.lineSeparator(), 2).nn.head.nn

val positionMarkerLine =
if pos.exists && pos.source.exists then
positionMarker(pos)
else ""
(if isLastTraceItem then EMPTY_PADDING else CONNECTING_INDENT)+ positionMarker(pos)
else
""

// always use the more precise trace location
if lastLineNum == pos.line then
if lastLineNum >= 0 && lastLineNum == pos.line then
lines.dropRightInPlace(1)

lines += (prefix + line + "\n" + positionMarkerLine)
val prefix = if isLastTraceItem then LAST_CHILD else CHILD
lines += (prefix + line + System.lineSeparator() + positionMarkerLine)

lastLineNum = pos.line
}
Expand All @@ -78,10 +86,10 @@ object Trace:
*/
private def positionMarker(pos: SourcePosition): String =
val trimmed = pos.source.lineContent(pos.start).takeWhile(c => c.isWhitespace).length
val padding = pos.startColumnPadding.substring(trimmed).nn + " "
val padding = pos.startColumnPadding.substring(trimmed).nn
val carets =
if (pos.startLine == pos.endLine)
"^" * math.max(1, pos.endColumn - pos.startColumn)
else "^"

s"$padding$carets\n"
s"$padding$carets" + System.lineSeparator()
26 changes: 13 additions & 13 deletions tests/init-global/neg/global-cycle1.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@
1 |object A { // error
| ^
| Cyclic initialization: object A -> object B -> object A. Calling trace:
| -> object A { // error [ global-cycle1.scala:1 ]
| ^
| -> val a: Int = B.b [ global-cycle1.scala:2 ]
| ^
| -> object B { [ global-cycle1.scala:5 ]
| ^
| -> val b: Int = A.a // error [ global-cycle1.scala:6 ]
| ^
| ├── object A { // error [ global-cycle1.scala:1 ]
| │ ^
| ├── val a: Int = B.b [ global-cycle1.scala:2 ]
| │ ^
| ├── object B { [ global-cycle1.scala:5 ]
| │ ^
| └── val b: Int = A.a // error [ global-cycle1.scala:6 ]
| ^
-- Error: tests/init-global/neg/global-cycle1.scala:6:17 ---------------------------------------------------------------
6 | val b: Int = A.a // error
| ^^^
| Access uninitialized field value a. Call trace:
| -> object B { [ global-cycle1.scala:5 ]
| ^
| -> val b: Int = A.a // error [ global-cycle1.scala:6 ]
| ^^^
| Access uninitialized field value a. Calling trace:
| ├── object B { [ global-cycle1.scala:5 ]
| │ ^
| └── val b: Int = A.a // error [ global-cycle1.scala:6 ]
| ^^^
18 changes: 9 additions & 9 deletions tests/init-global/neg/line-spacing.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
3 | B
4 | .s.length // error
| ^
| Access uninitialized field value s. Call trace:
| -> object B { [ line-spacing.scala:7 ]
| ^
| -> val s: String = s"${A.a}a" [ line-spacing.scala:8 ]
| ^^^
| -> def a: Int = [ line-spacing.scala:2 ]
| ^
| -> .s.length // error [ line-spacing.scala:4 ]
| ^
| Access uninitialized field value s. Calling trace:
| ├── object B { [ line-spacing.scala:7 ]
| │ ^
| ├── val s: String = s"${A.a}a" [ line-spacing.scala:8 ]
| │ ^^^
| ├── def a: Int = [ line-spacing.scala:2 ]
| │ ^
| └── .s.length // error [ line-spacing.scala:4 ]
| ^
14 changes: 7 additions & 7 deletions tests/init-global/neg/patmat-unapplySeq.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
8 | def apply(i: Int): Box = array(i) // error
| ^^^^^^^^
|Reading mutable state of object A during initialization of object B.
|Reading mutable state of other static objects is forbidden as it breaks initialization-time irrelevance. Calling trace:
|-> object B: [ patmat-unapplySeq.scala:15 ]
| ^
|-> case A(b) => [ patmat-unapplySeq.scala:17 ]
| ^^^^
|-> def apply(i: Int): Box = array(i) // error [ patmat-unapplySeq.scala:8 ]
| ^^^^^^^^
|Reading mutable state of other static objects is forbidden as it breaks initialization-time irrelevance. Calling trace:
|├── object B: [ patmat-unapplySeq.scala:15 ]
|│ ^
|├── case A(b) => [ patmat-unapplySeq.scala:17 ]
|│ ^^^^
|└── def apply(i: Int): Box = array(i) // error [ patmat-unapplySeq.scala:8 ]
| ^^^^^^^^
12 changes: 6 additions & 6 deletions tests/init/neg/closureLeak.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
11 | l.foreach(a => a.addX(this)) // error
| ^^^^^^^^^^^^^^^^^
|Could not verify that the method argument is transitively initialized (Hot). It was found to be a function where "this" is (the original object of type (class Outer) where initialization checking started). Only transitively initialized arguments may be passed to methods (except constructors). Calling trace:
|-> class Outer { [ closureLeak.scala:1 ]
| ^
|-> l.foreach(a => a.addX(this)) // error [ closureLeak.scala:11 ]
| ^^^^^^^^^^^^^^^^^
|├── class Outer { [ closureLeak.scala:1 ]
|│ ^
|└── l.foreach(a => a.addX(this)) // error [ closureLeak.scala:11 ]
| ^^^^^^^^^^^^^^^^^
|
|Promoting the value to transitively initialized (Hot) failed due to the following problem:
|Could not verify that the method argument is transitively initialized (Hot). It was found to be the original object of type (class Outer) where initialization checking started. Only transitively initialized arguments may be passed to methods (except constructors).
|Non initialized field(s): value p. Promotion trace:
|-> l.foreach(a => a.addX(this)) // error [ closureLeak.scala:11 ]
| ^^^^
|└── l.foreach(a => a.addX(this)) // error [ closureLeak.scala:11 ]
| ^^^^
32 changes: 16 additions & 16 deletions tests/init/neg/cycle-structure.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,29 @@
3 | val x = B(this) // error
| ^^^^^^^
| Problematic object instantiation: arg 1 is not transitively initialized (Hot). Calling trace:
| -> case class A(b: B) { [ cycle-structure.scala:1 ]
| ^
| -> val x = B(this) // error [ cycle-structure.scala:3 ]
| ^^^^^^^
| ├── case class A(b: B) { [ cycle-structure.scala:1 ]
| │ ^
| └── val x = B(this) // error [ cycle-structure.scala:3 ]
| ^^^^^^^
|
| It leads to the following error during object initialization:
| Access field value x on an uninitialized (Cold) object. Calling trace:
| -> case class B(a: A) { [ cycle-structure.scala:7 ]
| ^
| -> val x1 = a.x [ cycle-structure.scala:8 ]
| ^^^
| ├── case class B(a: A) { [ cycle-structure.scala:7 ]
| │ ^
| └── val x1 = a.x [ cycle-structure.scala:8 ]
| ^^^
-- Error: tests/init/neg/cycle-structure.scala:9:13 --------------------------------------------------------------------
9 | val x = A(this) // error
| ^^^^^^^
| Problematic object instantiation: arg 1 is not transitively initialized (Hot). Calling trace:
| -> case class B(a: A) { [ cycle-structure.scala:7 ]
| ^
| -> val x = A(this) // error [ cycle-structure.scala:9 ]
| ^^^^^^^
| ├── case class B(a: A) { [ cycle-structure.scala:7 ]
| │ ^
| └── val x = A(this) // error [ cycle-structure.scala:9 ]
| ^^^^^^^
|
| It leads to the following error during object initialization:
| Access field value x on an uninitialized (Cold) object. Calling trace:
| -> case class A(b: B) { [ cycle-structure.scala:1 ]
| ^
| -> val x1 = b.x [ cycle-structure.scala:2 ]
| ^^^
| ├── case class A(b: B) { [ cycle-structure.scala:1 ]
| │ ^
| └── val x1 = b.x [ cycle-structure.scala:2 ]
| ^^^
16 changes: 8 additions & 8 deletions tests/init/neg/default-this.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
| ^^^^^^^
|Could not verify that the method argument is transitively initialized (Hot). It was found to be the original object of type (class B) where initialization checking started. Only transitively initialized arguments may be passed to methods (except constructors).
|Non initialized field(s): value result. Calling trace:
|-> class B extends A { [ default-this.scala:6 ]
| ^
|-> val result = updateThenCompare(5) [ default-this.scala:11 ]
| ^^^^^^^^^^^^^^^^^^^^
|-> def updateThenCompare(c: Int): Boolean = { [ default-this.scala:7 ]
| ^
|-> compare() // error [ default-this.scala:9 ]
| ^^^^^^^
|├── class B extends A { [ default-this.scala:6 ]
|│ ^
|├── val result = updateThenCompare(5) [ default-this.scala:11 ]
|│ ^^^^^^^^^^^^^^^^^^^^
|├── def updateThenCompare(c: Int): Boolean = { [ default-this.scala:7 ]
|│ ^
|└── compare() // error [ default-this.scala:9 ]
| ^^^^^^^
16 changes: 8 additions & 8 deletions tests/init/neg/i15363.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
3 | val b = new B(this) // error
| ^^^^^^^^^^^
| Problematic object instantiation: arg 1 is not transitively initialized (Hot). Calling trace:
| -> class A: [ i15363.scala:1 ]
| ^
| -> val b = new B(this) // error [ i15363.scala:3 ]
| ^^^^^^^^^^^
| ├── class A: [ i15363.scala:1 ]
| │ ^
| └── val b = new B(this) // error [ i15363.scala:3 ]
| ^^^^^^^^^^^
|
| It leads to the following error during object initialization:
| Access field value m on an uninitialized (Cold) object. Calling trace:
| -> class B(a: A): [ i15363.scala:7 ]
| ^
| -> val x = a.m [ i15363.scala:8 ]
| ^^^
| ├── class B(a: A): [ i15363.scala:7 ]
| │ ^
| └── val x = a.m [ i15363.scala:8 ]
| ^^^
12 changes: 6 additions & 6 deletions tests/init/neg/i15459.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
| ^^^^
|Could not verify that the method argument is transitively initialized (Hot). It was found to be the original object of type (class Sub) where initialization checking started. Only transitively initialized arguments may be passed to methods (except constructors).
|Non initialized field(s): value b. Calling trace:
|-> class Sub extends Sup: [ i15459.scala:5 ]
| ^
|-> class Sup: [ i15459.scala:1 ]
| ^
|-> println(this) // error [ i15459.scala:3 ]
| ^^^^
|├── class Sub extends Sup: [ i15459.scala:5 ]
|│ ^
|├── class Sup: [ i15459.scala:1 ]
|│ ^
|└── println(this) // error [ i15459.scala:3 ]
| ^^^^
Loading