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

[Maurizio Cimadamore]

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.

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)

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

Maurizio

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