make private[this] if possible

[xuwei-k]

private (not private[this]) val and var generate accessor methods.
This change reduce class file size

[NthPortal]

In terms of the actual diff, I don't see any problems - it looks straightforward and unlikely to break anything to me. I'd be interested to hear what some of the core team think about always using private[this] in general though.

[floating-cat]

FYI, there is a similar pull request in Dotty last year.

[mkeskells]

This seems to be an optimisation that the compiler can perform automatically

Private fields can only be accessed by companions, and companions are required to be in the same compilation unit, so the compiler is aware if there is an accessor required.

The same optimisation can be applied to final vars, or effectively final vars (vars that are not declared as final in the code, but cannot be overridden, as they are in object, sealing etc.)
rorygraves#46 and rorygraves#47 are related

There is also interplay with the inliner and gitter, so we will need to have a performance run to ensure/fix any performance degradation, or (hopefully) claim the benefit.

Internally (in my day job) we have discussed making a scalafix rule to do this. Didn't get further than discussing it though 😞

[som-snytt]

I have long wanted private to mean private[this] and require private[C] for current semantics. It just occurred to me to wonder why the current syntax isn't private[this.type].

As an optimization, it breaks reflective access.

[mkeskells]

@som-snytt where is the definition of the reflexive it breaks?

I agree that it changes the methods generated(but that is sort of the point), but other patterns also generate methods for private[this] fields in some circumstances, or don't generate fields for some declared vals, etc

[som-snytt]

@mkeskells I just meant that a tool using reflection expecting an accessor will break. Dealing with fields introduces the funky space in the name, so filtering fields for those not the target of an accessor, etc, would make me sympathetic to a tool operating on accessors.

scala> class C { private[this] val i = 42 ; private val j = 17 }
defined class C

scala> typeOf[C].members.filter(_.isField).map(_.name)
res8: Iterable[$r.intp.global.Symbol#NameType] = List(j , i)

I think special accessors as implementation details are name-mangled?

[mkeskells]

@som-snytt my confusion grows. Why is there a 'funky space' apart from just trying to confuse those that forgot to check for that? I dont think this affects the discussion, but TIL :-)

My understanding is that the reflection information is derived from the pickled form stored in the scala attribute of the top level class, so this scala reflection view is unrelated to the java methods and fields that are generated or not.

I am proposing that the the emitted class makes the optimisation, not that the scala reflexive view is changed

Not that I understand the result below (another confusion - why final is treated differently) but IMO the scala view and java view of the reflected class are very different. Is i2 optimized away completely? As these are all unused constants, and not visible, then maybe it should generate no java fields or methods!!

Anyway back to the current views of the class from java and scala

scala> class Mike { private[this] val i = 42 ; private val j = 17; private[this] final val i2 = 42 ; private final val j2 = 17 }
defined class Mike

scala> classOf[Mike].getDeclaredFields
res9: Array[java.lang.reflect.Field] = Array(private final int Mike.i, private final int Mike.j)

scala> classOf[Mike].getDeclaredMethods
res10: Array[java.lang.reflect.Method] = Array(private int Mike.j(), private final int Mike.j2())

scala> import scala.reflect.runtime.universe._
typeOf[Mike].members

scala> res11: reflect.runtime.universe.MemberScope = Scopes(value j2, value j2, value i2, value j, value j, value i, constructor Mike, method $asInstanceOf, method $isInstanceOf, method synchronized, method ##, method !=, method ==, method ne, method eq, method notifyAll, method notify, method clone, method getClass, method hashCode, method toString, method equals, method wait, method wait, method wait, method finalize, method asInstanceOf, method isInstanceOf)

scala> typeOf[Mike].members.map(_.name)

res12: Iterable[reflect.runtime.universe.Symbol#NameType] = List(j2 , j2, i2, j , j, i, <init>, $asInstanceOf, $isInstanceOf, synchronized, $hash$hash, $bang$eq, $eq$eq, ne, eq, notifyAll, notify, clone, getClass, hashCode, toString, equals, wait, wait, wait, finalize, asInstanceOf, isInstanceOf)

[som-snytt]

Yes, final val without a type ascription is taken as a constant, per spec. It emits a field but doesn't init it, last I checked, years ago. Pre-Fields.

[mkeskells]

@som-snytt what I meant is that a private [this] val is final anyway - just another optimisation.
Also if the class is final the optimisation is not applied

scala> final class Mike2 { private[this] val i = 42 ; private val j = 17; private[this] final val i2 = 42 ; private final val j2 = 17 }
defined class Mike2

scala> classOf[Mike2].getDeclaredMethods
res19: Array[java.lang.reflect.Method] = Array(private int Mike2.j(), private final int Mike2.j2())

scala> classOf[Mike2].getDeclaredFields
res20: Array[java.lang.reflect.Field] = Array(private final int Mike2.i, private final int Mike2.j)

[lrytz]

For lazy vals it doesn't change anything in the bytecode, but we can keep them in anyway.