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

Always keep a field for private[this] var class parameters #9226

Merged
merged 2 commits into from Sep 28, 2020

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Sep 25, 2020

Reads of var class parameters in the constructor are not rewritten to
the constructor parameter, they keep referring to the field to ensure
potential field writes are reflected (this was fixed in #7688).

This means that the corresponding field cannot be elided even if it's
never written.

The alternative solution would be to detect private[this] var fields
that are only read in the constructor. However this is difficult to
implement, see #9143.

The second commit goes a step further by never eliding the field
for a private[this] var class parameter. Up for discussion.

Fixes scala/bug#12002 (the remaining part).

This needs to be backported to 2.12.x.

@lrytz
Copy link
Member Author

lrytz commented Sep 25, 2020

I'm personally in favor of the simpler solution, always keep the field. Even dotty agrees

➜  sandbox git:(t12002b) dotc ../test/files/run/t12002.scala && dotr Test
good boy!
List()
List()
List(private int C.x)
List(private int D.x)
List(private int E.x)
List(private int F.x)
List(private int G.x)
List(private final int H.x)
List(private int I.x)

@som-snytt
Copy link
Contributor

On one of the PRs, I mention that doti doesn't do it, but it wasn't clear what "inferred private[this]" would entail in doti.

@som-snytt
Copy link
Contributor

I agree there is not much benefit in this secret handshake. If user doesn't want a field, they should change their code.

The possible use case was if class var param was mutated only during constructor, but otherwise unused.

The test fail is that neg/t1503 with Nil.type.

Reads of `var` class parameters in the constructor are not rewritten to
the constructor parameter, they keep referring to the field to ensure
potential field writes are reflected (this was fixed in scala#7688).

This means that the corresponding field cannot be elided even if it's
never written.

The alternative solution would be to detect `private[this] var` fields
that are only read in the constructor. However this is difficult to
implement, see scala#9143.
After the previous commit, the field for a `private[this] var` class
parameter is only elided if the field is not used at all. This seems
not to be worth the effort.
@lrytz
Copy link
Member Author

lrytz commented Sep 28, 2020

The possible use case was if class var param was mutated only during constructor, but otherwise unused.

👍 but you can't do that with method parameters either, so no big loss and in a way it makes things more regular

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants