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

Fix #15363: Improve error messages for leaking of this #15364

Merged
merged 10 commits into from
Jun 16, 2022

Conversation

liufengyun
Copy link
Contributor

@liufengyun liufengyun commented Jun 3, 2022

Fix #15363: Improve error messages for leaking of this

class A:
  // should report one error here
  val b = new B(this) // error
  val m = 10
  val n = 20

class B(a: A):
  val x = a.m
  val y = a.n

For the code above, we now report the following error:

-- Error: tests/init/neg/i15363.scala:3:10 -----------------------------------------------------------------------------
3 |  val b = new B(this) // error
  |          ^^^^^^^^^^^
  |          Problematic object instantiation: arg 1 is not fully initialized. 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 on a value with an unknown initialization status. Calling trace:
  |          -> class B(a: A):	[ i15363.scala:7 ]
  |             ^
  |          -> val x = a.m	[ i15363.scala:8 ]
  |                     ^^^

@liufengyun liufengyun requested a review from olhotak June 3, 2022 05:52
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.

Overall a big improvement.

compiler/src/dotty/tools/dotc/transform/init/Errors.scala Outdated Show resolved Hide resolved
warm.callConstructor(ctor, argInfos2)
}
if errors.nonEmpty then
val error = UnsafeLeaking(trace.toVector, errors.head)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it guaranteed that all errors are uses of one of the passed non-hot values? Could there be an error in the called constructor that is unrelated to the passed arguments?

val multiple = argsIndices.size > 1
val part1 =
argsIndices.zipWithIndex.foldLeft("") { case (acc, (pos, i)) =>
val text1 = if pos == 0 then "the outer" else "arg " + pos.toString
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to print the class name of the outer in the error message.

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 pending a fix for #15374.

@liufengyun
Copy link
Contributor Author

@olhotak I created #15465 to track the issue so that we can merge this one to unblock other PRs.

@liufengyun liufengyun merged commit 61e57e6 into scala:main Jun 16, 2022
@liufengyun liufengyun deleted the fix-15363 branch June 16, 2022 20:27
@Kordyjan Kordyjan added this to the 3.2.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.

Improve error messages for leaking of this as constructor arguments
3 participants