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

No longer emit accessors for private fields #8286

Open
wants to merge 1 commit into
base: 2.13.x
from

Conversation

@lrytz
Copy link
Member

lrytz commented Jul 31, 2019

Except for value class and case class parameters.

For value class parameters, the accessor is used as the unbox method in
other classes. There are a few assumptions in the compiler for that method
being present.

Private case class parameters are excluded because an accessor is generated
anyway to support extraction. We could change the backend avoid going through
the accessor within the class.

Mark object fields static in CleanUp, because:

class C { def f = C.x }
object C { private val x = 0 }

The field x is made notPrivate to allow access from C. In the
backend, the field is emitted static. However, if only the backend
phase adds the static flag to the symbol, it's not there yet when
emitting C, and therefore the field access has the wrong opcode.

@scala-jenkins scala-jenkins added this to the 2.13.1 milestone Jul 31, 2019
@lrytz

This comment has been minimized.

Copy link
Member Author

lrytz commented Jul 31, 2019

Linting for unused privates would need to be fixed, as now there are no more unused accessors, only the field which is accessed in the constructor - should be doable, as linting works correctly for private[this].

@lrytz lrytz force-pushed the lrytz:privateNoAccessors branch from bcc4864 to 3e1c68a Jul 31, 2019
For value class parameters, the accessor is used as the `unbox` method in
other classes. There are a few assumptions in the compiler for that method
being present.

Private case class parameters are excluded because an accessor is generated
anyway to support extraction. We could change the backend avoid going through
the accessor within the class.

Mark object fields static in CleanUp, because:

```
class C { def f = C.x }
object C { private val x = 0 }
```

The field `x` is made `notPrivate` to allow access from C. In the
backend, the field is emitted static. However, if only the backend
phase adds the `static` flag to the symbol, it's not there yet when
emitting `C`, and therefore the field access has the wrong opcode.
@lrytz lrytz force-pushed the lrytz:privateNoAccessors branch from 3e1c68a to 75aa4bb Jul 31, 2019
@lrytz lrytz requested a review from retronym Jul 31, 2019
@retronym

This comment has been minimized.

Copy link
Member

retronym commented Aug 2, 2019

I can imagine that this could break some reflection based frameworks that are looking for annotated getter methods that we no longer emit. So maybe we need to expose this as a compiler setting. One of the options could be to emit them for annotated fields.

@lrytz lrytz modified the milestones: 2.13.1, 2.14.0-M1 Aug 16, 2019
@lrytz lrytz marked this pull request as ready for review Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.