[lworld] RFR: 8330582: [lworld] The JVM must enforce new field modifiers rules [v2]

[Frederic Parain]

On Mon, 22 Apr 2024 18:19:41 GMT, Dan Heidinga <heidinga at openjdk.org> wrote:

>> Frederic Parain has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix fields access flags filtering
>
> src/hotspot/share/classfile/classFileParser.cpp line 4800:
> 
>> 4798:         is_illegal = true;
>> 4799:       } else if (supports_inline_types()) {
>> 4800:         if (!is_identity_class && !is_static && !is_strict) {
> 
> Suggestion:
> 
>         if (!is_identity_class && !is_static && !is_strict) {
>             /* non-static value class fields must be be strict */

Fixed as suggested.

> src/hotspot/share/classfile/classFileParser.cpp line 4802:
> 
>> 4800:         if (!is_identity_class && !is_static && !is_strict) {
>> 4801:           is_illegal = true;
>> 4802:         } else if (is_abstract && !is_identity_class && !is_static) {
> 
> Suggestion:
> 
>         } else if (is_abstract && !is_identity_class && !is_static) {
>             /* abstract value classes can only have static fields */
> 
> If I'm reading that correctly, we're not yet allowing value classes to inherit instance fields.  Should this have a TODO so we come back and support as per JEP 401 later?

I'll start working on value classes field inheritance immediately after the the first nullable flat field patch is pushed (hopefully this week).

> src/hotspot/share/include/jvm_constants.h line 49:
> 
>> 47:                                         JVM_ACC_ENUM | \
>> 48:                                         JVM_ACC_SYNTHETIC | \
>> 49:                                         JVM_ACC_STRICT)
> 
> Should we flip the order of these so they appear in bit order and match the METHOD_MODIFIERS order as well?
> 
> Suggestion:
> 
>                                         JVM_ACC_STRICT | \
>                                         JVM_ACC_SYNTHETIC)

Fixed as suggested.

-------------

PR Review Comment: https://git.openjdk.org/valhalla/pull/1076#discussion_r1575317584
PR Review Comment: https://git.openjdk.org/valhalla/pull/1076#discussion_r1575320804
PR Review Comment: https://git.openjdk.org/valhalla/pull/1076#discussion_r1575318986