Should final fields in records be trusted or not trusted in 16?

Thu Dec 10 14:11:31 UTC 2020

----- Mail original -----
> De: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>
> À: "Chris Hegarty" <chris.hegarty at oracle.com>, "Remi Forax" <forax at univ-mlv.fr>, "amber-spec-experts"
> <amber-spec-experts at openjdk.java.net>
> Cc: "mandy chung" <mandy.chung at oracle.com>
> Envoyé: Jeudi 10 Décembre 2020 14:22:36
> Objet: Re: Should final fields in records be trusted or not trusted in 16?

> What Chris says.
> 
> The immediate problem is that the Javadoc for Field::set is incorrect.
> Fixing it by mentioning presence of a bytecode attribute somewhere is
> not really a fix.
> 
> Having better treatment for final fields for more classes is a noble
> goal - having restricted classes like records which act as pure data
> carrier is another noble goal.
> 
> What is not noble IMHO, is the attempt to reuse the record classfile
> machinery (e.g. record attribute) to improve final field semantics even
> in things that are not records.

But what a record is ? For the VM, not for the language.
If the definition is a class that contains a record attribute which is the current definition for the VM, then i don't see a problem if Scala Tuples or an immutable Kotlin data types are using the record Attribute.

The real issue is that currently Field.set() is using the reflection API definition of a record instead of using the VM definition.

> 
> Ideally there would be TWO attributes: the Record attribute and the
> PleaseMakeMyFinalFieldsFast attribute. But we don't have the latter
> (yet), and it's outside the scope of the work Chris and Mandy proposed
> (again, mainly about fixing the impl so that what the javadoc says is
> correct) to address the latter point. Which leaves us two options:
> 
> * revert the changes which allowed the VM to trust record fields
> * keep trusting record fields, but on a more solid definition of what a
> record is (which is in line with what the JLS, and the reflection API say)

There is a third choice see above.

Forcing the semantics of Java into the VM is always a bad idea.
We don't do that for any other constructs, lambda, enums, Java anonymous classes, etc.
Worst, we already know that an inline record is not a record from the JLS POV.

For Java 16, given that we are late in the process, i see no problem if the VM doesn't trust record fields as a temporary patch, if it's easier than to have Field.set() to ask if a class is a plain class or not. It's far better than having the the JLS definition of a record bolted into the VM.

> 
> Any other solution IMHO seems more like an hack to me.

I disagree, having the VM behaving strictly like Java is a hack. Microsoft has done that with .Net generics which are C# generics. I don't see the point of repeating the errors of the past. 

> 
> Maurizio

Rémi

> 
> On 09/12/2020 16:58, Chris Hegarty wrote:
>> I see that the thread [1] tailed off without any real conclusion or
>> consensus.
>>
>> Getting to a place where it is simple to add new kinds of classes whose
>> non-static final fields are trusted is a worthy goal. I just question if
>> doing so with a record attribute is the right move. Instead we can
>> consider how best that could be achieved orthogonal to records.
>>
>> I see Mandy's PR as an intermediate step towards that goal. For now, we
>> limit TNSFF to records and hidden classes, but eventually this can be
>> superseded by a predicate on j.l.Class (or some such), which encompasses
>> the aforementioned kinds of classes and possibly others.
>>
>> We have a material specification issue in Java 16 (as Mandy describes) -
>> how to specify java.lang.reflect.Field::set. Mandy's PR resolves this
>> issue while retaining TNSFF for record classes. This leaves the door
>> open to further evolution to get to the above goal.
>>
>> -Chris.
>>
>>> On 9 Dec 2020, at 10:39, Remi Forax <forax at univ-mlv.fr> wrote:
>>>
>>> forwarded to amber-spec-experts
>>>
>>> Rémi
>>>
>>> ----- Mail transféré -----
>>> De: "mandy chung" <mandy.chung at oracle.com>
>>> À: "amber-dev" <amber-dev at openjdk.java.net>
>>> Envoyé: Mercredi 9 Décembre 2020 05:00:03
>>> Objet: Should final fields in records be trusted or not trusted in 16?
>>>
>>> I need your help, amber experts, in understanding the conclusion on the
>>> amber-spec-experts discussion [1].  It isn't clear to me what it's
>>> agreed to do in Java SE 16. Remi raised in PR for JDK-8257596 [2] and so
>>> your clarification would help.  PR #1706 intends to fix the regression
>>> introduced by JDK-8255342 that removes non-specified JVM checks on
>>> classes with RecordComponents attributes.  This does not conflict with
>>> the work to implement the true TNSFF for all classes like JDK-8233873.
>>>
>>> One way I read [1] is that it's agreed to revisit the current approach
>>> [3] that makes final fields in record classes "read-only" by reflection
>>> and JIT optimization to trust final fields in records (note that JIT
>>> optimization is implementation-specific). Instead all final field values
>>> should be trusted as a constant (see JDK-8233873).
>>>
>>> If this is the agreement, I see two options for JDK 16:
>>> 1. Keep JDK-8247444 and fix the regression as proposed by PR #1706 [2]
>>> 2. Backout JDK-8247444 [4].  This involves spec change and we shall act
>>> on it quickly.
>>>
>>> Making all final field values trusted as a constant will be a separate
>>> enhancement regardless of which option it goes.
>>>
>>> Please clarify.
>>>
>>> Mandy
>>> [1]
>>> https://mail.openjdk.java.net/pipermail/amber-spec-experts/2020-November/002630.html
>>> [2] https://github.com/openjdk/jdk/pull/1706
>>> [3] https://bugs.openjdk.java.net/browse/JDK-8247444
> >> [4] https://github.com/openjdk/jdk/compare/master...mlchung:backout-8247444