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.

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

Conversation

q-ata
Copy link
Contributor

@q-ata q-ata commented Mar 1, 2023

Rewords some warning messages produced by the -Ysafe-init flag so that they are better understood by the user.

Addresses Issue #15836

Copy link
Contributor

@olhotak olhotak left a comment

Choose a reason for hiding this comment

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

Please update .check files not only so the tests pass, but also so we can review the full error messages in context.

@@ -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 promoted to transitively initialized (Hot)") }
Copy link
Contributor

Choose a reason for hiding this comment

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

@liufengyun Should we consider saying something like "proven" instead of "promoted"? e.g. "Argument cannot be proven to be transitively initialized"

Copy link
Contributor

Choose a reason for hiding this comment

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

In the documentation, we say "promotion" and have a definition for it. It seems to me "prove" is better than "promoted". We can see if we can change the definition in the docs as well --- something like "provably hot".

| ^
| -> 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.

@olhotak olhotak requested review from liufengyun and removed request for liufengyun March 7, 2023 14:33
@q-ata q-ata requested a review from liufengyun March 9, 2023 01:43
Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

A good improvement, thank you @q-ata 🎉

Copy link
Contributor

@olhotak olhotak left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

@olhotak olhotak merged commit a569057 into scala:main Mar 27, 2023
@Kordyjan Kordyjan added this to the 3.3.1 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants