scala3
scala3 copied to clipboard
scala
Error message about initialization should be more understandable
Compiler version
3.2.0-RC3
Minimized example
This is the first initialization error messsge I have encountered in my own code "in the wild".
error] -- Error: /Users/odersky/workspace/dotty/compiler/src/dotty/tools/dotc/cc/CaptureSet.scala:87:16
[error] 87 | cs.addSuper(this)(using ctx, UnrecordedState)
[error] | ^^^^
[error] |Cannot prove the method argument is hot. Only hot values are safe to leak.
[error] |Found = ThisRef[class Mapped].
[error] |Non initialized field(s): value stack. Calling trace:
[error] |-> class Mapped private[CaptureSet] [ CaptureSet.scala:436 ]
[error] | ^
[error] |-> abstract class DerivedVar(initialElems: Refs)(using @constructorOnly ctx: Context) [ CaptureSet.scala:423 ]
[error] | ^
[error] |-> addSub(source) [ CaptureSet.scala:427 ]
[error] | ^^^^^^^^^^^^^^
[error] |-> protected def addSub(cs: CaptureSet)(using Context): this.type = [ CaptureSet.scala:86 ]
[error] | ^
[error] |-> cs.addSuper(this)(using ctx, UnrecordedState) [ CaptureSet.scala:87 ]
[error] | ^^^^
[error] one error found
The message should be clearer:
- 'Hot" is expert terminology from the paper. Nobody outside knows what that is. Use "fully initialized" instead.
- The rest of the message is also hard to parse. What is
ThisRef[class Mapped]? What is the significance of the calling trace. As someone who knows the theory I can guess, but for a non-expert this is hard to parse.
This is the first initialization error messsge I have encountered in my own code "in the wild".
Welcome to the initialized world😉 We fixed a bunch of initialization warnings in the compiler already for bootstrapping. Luckily the one you encountered is easy to fix.
'Hot" is expert terminology from the paper. Nobody outside knows what that is. Use "fully initialized" instead.
We recently changed "fully initialized" to "hot" with the intention to familiarize the programmer with the terminology.
The reason is that we soon run out of terminology for displaying helpful & precise information in debugging complex initialization warnings in the compiler. To facilitate reasoning & debugging about safe initialization, we make the concepts and rules explicit in the documentation.
We need to revisit that decision. Maybe we can write "fully initialized (hot)", which is more friendly.
What is the significance of the calling trace
The stack trace shows how the problematic code is reached from the constructor.
We need to revisit that decision. Maybe we can write "fully initialized (hot)", which is more friendly.
Yes, I think that would be better.
The stack trace shows how the problematic code is reached from the constructor.
That's what I guessed. But for a newcomer some explanation what it is would be helpful.
@liufengyun and I recently tried to make the error messages more specific. This was necessary to report to a user what actually went wrong in complicated cases. But to make the messages more specific, we needed to define various terms and concepts from the analysis and explain a bit about how the analysis works. That explanation/definitions/documentation is currently at: https://dotty.epfl.ch/docs/reference/other-new-features/safe-initialization.html There are too many concepts and details in that document to include them all in each error message.
One easy way to improve the error messages would be to add a link to this page in each error message, e.g. `For a detailed explanation of the terminology in this error message, see https://dotty.epfl.ch/docs/reference/other-new-features/safe-initialization.html ."
A further improvement could be to rename the terms defined there using words that have more of an inherent meaning. For example, "hot" could renamed to "transitively initialized" (which I think has a more explicit meaning than "fully initialized"). But we couldn't think of similar terms for other such concepts. I think the following are the most significant:
ThisRef - The analysis starts at the beginning of the constructor of each class assuming all its parameters are hot/transitively initialized. The execution of that constructor may create other objects and therefore enter the constructors of those objects. ThisRef refers to the object at whose constructor the analysis started (as opposed to the various other objects being constructed during the construction of ThisRef). I can't think of a good term with an intuitive meaning that would convey this concept all in one or two words.
Warm - We often define this as "an object such that all its fields have been assigned references, but it may not be transitively initialized: those references may be to objects that are themselves not yet initialized." But for the purpose of the error messages, a different aspect of Warm is relevant: these are the objects other than ThisRef that might not yet be transitively initialized. That is, these are the objects that are created during the execution of the constructor of ThisRef. However, of those objects, it excludes those that the analysis can prove to be transitively initialized (since those cannot cause any initialization problems). Again, I don't have a good way to convey this concept in one or two words in an error message.
I think having good brief wording for Hot, ThisRef, and Warm as explained above would go a long way for most of the error messages. But then there are more concepts in the document that are less common, but still come up sometimes, and are difficult to explain briefly. One particularly tricky example is "effectively hot".
We should design error messages so that they are intuitively understandable at least to some degree without reference to a text. I think "transitively initialized" is much better than "hot". In particular, one can google it.
The stack trace shows how the problematic code is reached from the constructor.
That's what I guessed. But for a newcomer some explanation what it is would be helpful.
Instead of "Calling trace" we can show "Calling trace for creating objects of class X", which might help understand.
Here is an example of how we could rephrase this particular error message:
In a state when all objects are initialized, an object of class Mapped is constructed:
|-> class Mapped private[CaptureSet] [ CaptureSet.scala:436 ]
The constructor calls the constructor of the superclass DerivedVar:
|-> abstract class DerivedVar(initialElems: Refs)(using @constructorOnly ctx: Context) [ CaptureSet.scala:423 ]
Execution reaches the following method call:
|-> addSub(source) [ CaptureSet.scala:427 ]
The method call calls the following method:
|-> protected def addSub(cs: CaptureSet)(using Context): this.type = [ CaptureSet.scala:86 ]
Execution reaches the following method call:
|-> cs.addSuper(this)(using ctx, UnrecordedState) [ CaptureSet.scala:87 ]
| ^^^^
Argument 0 of the call cannot be proved to be transitively initialized. Only transitively initialized arguments may be passed to methods outside the analyzed initialization region.
The value of argument 0 is the original object of class Mapped that started the trace. It cannot be proven to be transitively initialized because its field stack is not yet assigned.
By starting every warning message with the first line ("In a state ..., an object of class Mapped is constructed"), this allows us to say "ThisRef" using the phrase "the original object of class Mapped that started the trace".
We could do something along these lines in general for all of the error messages. The messages would be somewhat more verbose than they are now and it would take some implementation effort to add all the explanatory text to the trace, but they would be more self-explanatory, especially to newcomers.
@olhotak I think the proposed revised version is friendly and quite understandable.