Skip to content

Reword error messages in initialization checker to be more user friendly. #17030

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

Merged
merged 7 commits into from
Mar 27, 2023
Merged

Reword error messages in initialization checker to be more user friendly. #17030

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
da1b028
Reword error messages to be more user friendly.
q-ata Mar 1, 2023
585409a
Update check files, minor wording changes to error messages
q-ata Mar 6, 2023
4cc629e
Reword the stringified versions of some objects
q-ata Mar 7, 2023
6f22980
Add parentheses to make warnings more clear
q-ata Mar 7, 2023
36298a2
Address feedback.
q-ata Mar 9, 2023
603822e
Make warning message more accurate.
q-ata Mar 13, 2023
8af1b29
Reformat the output when printing a function to console in init checker
q-ata Mar 26, 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
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/dotc/transform/init/Errors.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ object Errors:

case class AccessCold(field: Symbol)(val trace: Trace) extends Error:
def show(using Context): String =
"Access field " + field.show + " on a cold object." + stacktrace
"Access field " + field.show + " on an uninitialized (Cold) object." + stacktrace

case class CallCold(meth: Symbol)(val trace: Trace) extends Error:
def show(using Context): String =
"Call method " + meth.show + " on a cold object." + stacktrace
"Call method " + meth.show + " on an uninitialized (Cold) object." + stacktrace

case class CallUnknown(meth: Symbol)(val trace: Trace) extends Error:
def show(using Context): String =
Expand All @@ -62,7 +62,7 @@ object Errors:
case class UnsafePromotion(msg: String, error: Error)(val trace: Trace) extends Error:
def show(using Context): String =
msg + stacktrace + "\n" +
"Promoting the value to hot (transitively initialized) failed due to the following problem:\n" + {
"Promoting the value to transitively initialized (Hot) failed due to the following problem:\n" + {
val ctx2 = ctx.withProperty(IsFromPromotion, Some(true))
error.show(using ctx2)
}
Expand Down Expand Up @@ -96,5 +96,5 @@ object Errors:
acc + text2
}
val verb = if multiple then " are " else " is "
val adjective = "not hot (transitively initialized)."
val adjective = "not transitively initialized (Hot)."
subject + verb + adjective
36 changes: 20 additions & 16 deletions compiler/src/dotty/tools/dotc/transform/init/Semantic.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,18 @@ object Semantic:
sealed abstract class Value:
def show(using Context): String = this match
case ThisRef(klass) =>
"ThisRef[" + klass.show + "]"
"the original object of type \"" + klass.show + "\" that started this trace"
case Warm(klass, outer, ctor, args) =>
val argsText = if args.nonEmpty then ", args = " + args.map(_.show).mkString("(", ", ", ")") else ""
"Warm[" + klass.show + "] { outer = " + outer.show + argsText + " }"
"an initialized (Warm) object of type " + klass.show + "{ outer = " + outer.show + argsText + " }"
case Fun(expr, thisV, klass) =>
"Fun { this = " + thisV.show + ", owner = " + klass.show + " }"
"a function { this = " + thisV.show + ", owner = " + klass.show + " }"
case RefSet(values) =>
values.map(_.show).mkString("Set { ", ", ", " }")
case _ =>
this.toString()
case Hot =>
"a transitively initialized (Hot) object"
case Cold =>
"an uninitialized (Cold) object"

def isHot = this == Hot
def isCold = this == Cold
Expand Down Expand Up @@ -470,7 +472,7 @@ object Semantic:
def widenArg: Contextual[Value] =
a match
case _: Ref | _: Fun =>
val hasError = Reporter.hasErrors { a.promote("Argument cannot be promoted to hot") }
val hasError = Reporter.hasErrors { a.promote("Argument cannot be proven to be transitively initialized (Hot)") }
if hasError then Cold else Hot

case RefSet(refs) =>
Expand Down Expand Up @@ -707,7 +709,9 @@ object Semantic:
// no source code available
promoteArgs()
// try promoting the receiver as last resort
val hasErrors = Reporter.hasErrors { ref.promote("try promote value to hot") }
val hasErrors = Reporter.hasErrors {
ref.promote(ref.show + " has no source code and could not be proven to be transitively initialized (Hot).")
}
if hasErrors then
val error = CallUnknown(target)(trace)
reporter.report(error)
Expand Down Expand Up @@ -992,7 +996,7 @@ object Semantic:
eval(body, thisV, klass)
}
given Trace = Trace.empty.add(body)
res.promote("The function return value is not hot. Found = " + res.show + ".")
res.promote("Only transitively initialized (Hot) values can be returned by functions. The function " + fun.show + " returns " + res.show + ".")
}
if errors.nonEmpty then
reporter.report(UnsafePromotion(msg, errors.head)(trace))
Expand Down Expand Up @@ -1036,7 +1040,7 @@ object Semantic:
//
// This invariant holds because of the Scala/Java/JVM restriction that we cannot use `this` in super constructor calls.
if subClassSegmentHot && !isHotSegment then
report.error("[Internal error] Expect current segment to hot in promotion, current klass = " + klass.show +
report.error("[Internal error] Expect current segment to be transitively initialized (Hot) in promotion, current klass = " + klass.show +
", subclass = " + subClass.show + Trace.show, Trace.position)

// If the outer and parameters of a class are all hot, then accessing fields and methods of the current
Expand All @@ -1053,12 +1057,12 @@ object Semantic:
val args = member.info.paramInfoss.flatten.map(_ => new ArgInfo(Hot: Value, Trace.empty))
val res = warm.call(member, args, receiver = warm.klass.typeRef, superType = NoType)
withTrace(trace.add(member.defTree)) {
res.promote("Cannot prove that the return value of " + member.show + " is hot. Found = " + res.show + ".")
res.promote("Could not verify that the return value of " + member.show + " is transitively initialized (Hot). It was found to be " + res.show + ".")
}
else
val res = warm.select(member, receiver = warm.klass.typeRef)
withTrace(trace.add(member.defTree)) {
res.promote("Cannot prove that the field " + member.show + " is hot. Found = " + res.show + ".")
res.promote("Could not verify that the field " + member.show + " is transitively initialized (Hot). It was found to be " + res.show + ".")
}
end for

Expand Down Expand Up @@ -1144,7 +1148,7 @@ object Semantic:

extension (arg: ArgInfo)
def promote: Contextual[Unit] = withTrace(arg.trace) {
arg.value.promote("Cannot prove the method argument is hot. Only hot values are safe to leak.\nFound = " + arg.value.show + ".")
arg.value.promote("Could not verify that the method argument is transitively initialized (Hot). It was found to be " + arg.value.show + ". Only transitively initialized arguments may be passed to methods outside the analyzed initialization region.")
}

/** Evaluate an expression with the given value for `this` in a given class `klass`
Expand Down Expand Up @@ -1285,12 +1289,12 @@ object Semantic:
eval(qual, thisV, klass)
val res = eval(rhs, thisV, klass)
extendTrace(expr) {
res.ensureHot("The RHS of reassignment must be hot. Found = " + res.show + ". ")
res.ensureHot("The RHS of reassignment must be transitively initialized (Hot). It was found to be " + res.show + ". ")
}
case id: Ident =>
val res = eval(rhs, thisV, klass)
extendTrace(expr) {
res.ensureHot("The RHS of reassignment must be hot. Found = " + res.show + ". ")
res.ensureHot("The RHS of reassignment must be transitively initialized (Hot). It was found to be " + res.show + ". ")
}

case closureDef(ddef) =>
Expand All @@ -1313,14 +1317,14 @@ object Semantic:
case Match(selector, cases) =>
val res = eval(selector, thisV, klass)
extendTrace(selector) {
res.ensureHot("The value to be matched needs to be hot. Found = " + res.show + ". ")
res.ensureHot("The value to be matched needs to be transitively initialized (Hot). It was found to be " + res.show + ". ")
}
eval(cases.map(_.body), thisV, klass).join

case Return(expr, from) =>
val res = eval(expr, thisV, klass)
extendTrace(expr) {
res.ensureHot("return expression must be hot. Found = " + res.show + ". ")
res.ensureHot("return expression must be transitively initialized (Hot). It was found to be " + res.show + ". ")
}

case WhileDo(cond, body) =>
Expand Down
22 changes: 10 additions & 12 deletions tests/init/neg/closureLeak.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
-- Error: tests/init/neg/closureLeak.scala:11:14 -----------------------------------------------------------------------
11 | l.foreach(a => a.addX(this)) // error
| ^^^^^^^^^^^^^^^^^
| Cannot prove the method argument is hot. Only hot values are safe to leak.
| Found = Fun { this = ThisRef[class Outer], owner = class Outer }. Calling trace:
| -> class Outer { [ closureLeak.scala:1 ]
| ^
| -> l.foreach(a => a.addX(this)) // error [ closureLeak.scala:11 ]
| ^^^^^^^^^^^^^^^^^
|Could not verify that the method argument is transitively initialized (Hot). It was found to be a function { this = the original object of type "class Outer" that started this trace, owner = class Outer }. Only transitively initialized arguments may be passed to methods outside the analyzed initialization region. Calling trace:
|-> class Outer { [ closureLeak.scala:1 ]
| ^
|-> l.foreach(a => a.addX(this)) // error [ closureLeak.scala:11 ]
| ^^^^^^^^^^^^^^^^^
|
| Promoting the value to hot (transitively initialized) failed due to the following problem:
| Cannot prove the method argument is hot. Only hot values are safe to leak.
| Found = ThisRef[class Outer].
| Non initialized field(s): value p. Promotion trace:
| -> 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" that started this trace. Only transitively initialized arguments may be passed to methods outside the analyzed initialization region.
|Non initialized field(s): value p. Promotion trace:
|-> l.foreach(a => a.addX(this)) // error [ closureLeak.scala:11 ]
| ^^^^
8 changes: 4 additions & 4 deletions tests/init/neg/cycle-structure.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
-- Error: tests/init/neg/cycle-structure.scala:3:13 --------------------------------------------------------------------
3 | val x = B(this) // error
| ^^^^^^^
| Problematic object instantiation: arg 1 is not hot (transitively initialized). Calling trace:
| 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 ]
| ^^^^^^^
|
| It leads to the following error during object initialization:
| Access field value x on a cold object. Calling trace:
| 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 ]
| ^^^
-- Error: tests/init/neg/cycle-structure.scala:9:13 --------------------------------------------------------------------
9 | val x = A(this) // error
| ^^^^^^^
| Problematic object instantiation: arg 1 is not hot (transitively initialized). Calling trace:
| 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 ]
| ^^^^^^^
|
| It leads to the following error during object initialization:
| Access field value x on a cold object. Calling trace:
| 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 ]
Expand Down
21 changes: 10 additions & 11 deletions tests/init/neg/default-this.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
-- Error: tests/init/neg/default-this.scala:9:8 ------------------------------------------------------------------------
9 | compare() // error
| ^^^^^^^
| Cannot prove the method argument is hot. Only hot values are safe to leak.
| Found = ThisRef[class B].
| 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 ]
| ^^^^^^^
|Could not verify that the method argument is transitively initialized (Hot). It was found to be the original object of type "class B" that started this trace. Only transitively initialized arguments may be passed to methods outside the analyzed initialization region.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is problematic because the method doesn't take any arguments. Is the error about the receiver of the method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function signature is def compare(c: Int = 5, a: A = this): Boolean. It could possibly be referring to the default A argument.

|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 ]
| ^^^^^^^
4 changes: 2 additions & 2 deletions tests/init/neg/i15363.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
-- Error: tests/init/neg/i15363.scala:3:10 -----------------------------------------------------------------------------
3 | val b = new B(this) // error
| ^^^^^^^^^^^
| Problematic object instantiation: arg 1 is not hot (transitively initialized). Calling trace:
| 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 ]
| ^^^^^^^^^^^
|
| It leads to the following error during object initialization:
| Access field value m on a cold object. Calling trace:
| 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 ]
Expand Down
17 changes: 8 additions & 9 deletions tests/init/neg/i15459.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
-- Error: tests/init/neg/i15459.scala:3:10 -----------------------------------------------------------------------------
3 | println(this) // error
| ^^^^
| Cannot prove the method argument is hot. Only hot values are safe to leak.
| Found = ThisRef[class Sub].
| 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 ]
| ^^^^
|Could not verify that the method argument is transitively initialized (Hot). It was found to be the original object of type "class Sub" that started this trace. Only transitively initialized arguments may be passed to methods outside the analyzed initialization region.
|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 ]
| ^^^^
26 changes: 13 additions & 13 deletions tests/init/neg/inherit-non-hot.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
-- Error: tests/init/neg/inherit-non-hot.scala:6:32 --------------------------------------------------------------------
6 | if b == null then b = new B(this) // error
| ^^^^^^^^^^^^^^^
| The RHS of reassignment must be hot. Found = Warm[class B] { outer = Hot, args = (Cold) }. Calling trace:
| -> class C extends A { [ inherit-non-hot.scala:15 ]
| ^
| -> val bAgain = toB.getBAgain [ inherit-non-hot.scala:16 ]
| ^^^
| -> def toB: B = [ inherit-non-hot.scala:5 ]
| ^
| -> if b == null then b = new B(this) // error [ inherit-non-hot.scala:6 ]
| ^^^^^^^^^^^^^^^
|The RHS of reassignment must be transitively initialized (Hot). It was found to be an initialized (Warm) object of type class B{ outer = a transitively initialized (Hot) object, args = (an uninitialized (Cold) object) }. Calling trace:
|-> class C extends A { [ inherit-non-hot.scala:15 ]
| ^
|-> val bAgain = toB.getBAgain [ inherit-non-hot.scala:16 ]
| ^^^
|-> def toB: B = [ inherit-non-hot.scala:5 ]
| ^
|-> if b == null then b = new B(this) // error [ inherit-non-hot.scala:6 ]
| ^^^^^^^^^^^^^^^
|
| Promoting the value to hot (transitively initialized) failed due to the following problem:
| Cannot prove that the field value a is hot. Found = Cold. Promotion trace:
| -> class B(a: A) { [ inherit-non-hot.scala:10 ]
| ^^^^
|Promoting the value to transitively initialized (Hot) failed due to the following problem:
|Could not verify that the field value a is transitively initialized (Hot). It was found to be an uninitialized (Cold) object. Promotion trace:
|-> class B(a: A) { [ inherit-non-hot.scala:10 ]
| ^^^^
17 changes: 8 additions & 9 deletions tests/init/neg/inlined-method.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
-- Error: tests/init/neg/inlined-method.scala:8:45 ---------------------------------------------------------------------
8 | scala.runtime.Scala3RunTime.assertFailed(message) // error
| ^^^^^^^
| Cannot prove the method argument is hot. Only hot values are safe to leak.
| Found = ThisRef[class InlineError].
| Non initialized field(s): value v. Calling trace:
| -> class InlineError { [ inlined-method.scala:1 ]
| ^
| -> Assertion.failAssert(this) [ inlined-method.scala:2 ]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
| -> scala.runtime.Scala3RunTime.assertFailed(message) // error [ inlined-method.scala:8 ]
| ^^^^^^^
|Could not verify that the method argument is transitively initialized (Hot). It was found to be the original object of type "class InlineError" that started this trace. Only transitively initialized arguments may be passed to methods outside the analyzed initialization region.
|Non initialized field(s): value v. Calling trace:
|-> class InlineError { [ inlined-method.scala:1 ]
| ^
|-> Assertion.failAssert(this) [ inlined-method.scala:2 ]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|-> scala.runtime.Scala3RunTime.assertFailed(message) // error [ inlined-method.scala:8 ]
| ^^^^^^^
13 changes: 6 additions & 7 deletions tests/init/neg/inner-first.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
-- Error: tests/init/neg/inner-first.scala:3:12 ------------------------------------------------------------------------
3 | println(this) // error
| ^^^^
| Cannot prove the method argument is hot. Only hot values are safe to leak.
| Found = ThisRef[class B].
| Non initialized field(s): value n. Calling trace:
| -> class B: [ inner-first.scala:2 ]
| ^
| -> println(this) // error [ inner-first.scala:3 ]
| ^^^^
|Could not verify that the method argument is transitively initialized (Hot). It was found to be the original object of type "class B" that started this trace. Only transitively initialized arguments may be passed to methods outside the analyzed initialization region.
|Non initialized field(s): value n. Calling trace:
|-> class B: [ inner-first.scala:2 ]
| ^
|-> println(this) // error [ inner-first.scala:3 ]
| ^^^^
Loading