[lworld] RFR: 8364095: [lworld] AccessFlags location and release need futher work for Valhalla (IDENTITY and SUPER) [v2]

[Chen Liang]

On Mon, 1 Sep 2025 11:01:20 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Years
>
> src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line 348:
> 
>> 346:                         .build(proxyDesc, clb -> {
>> 347:             clb.withSuperclass(CD_Object)
>> 348:                .withFlags(ACC_SUPER | ACC_FINAL | ACC_SYNTHETIC)
> 
> I presume this change is because the classfile API will be able to filter out ACC_SUPER based on the target version?

No, because the old code is wrong - the flags are no-ops if the class file version is not latest release's preview (by default it is latest release.0)

> src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 461:
> 
>> 459:          * @jvms 4.1 The {@code ClassFile} Structure
>> 460:          */
>> 461:         CLASS(ACC_PUBLIC | ACC_FINAL | ACC_IDENTITY |
> 
> Shouldn't we have a mapping here for `JDK N - 1` that has ACC_SUPER instead of ACC_IDENTITY ? For inner classes this is not needed because inner classes could never have ACC_SUPER -- but regular classes could?

This is just the bit masks, where ACC_SUPER is equivalent to ACC_IDENTITY. So 0x20 is always there in classes, but newly introduced to inner classes, so yes.

> src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java line 3348:
> 
>> 3346:             flags |= MODULE;
>> 3347:         }
>> 3348:         if (((flags & ACC_IDENTITY) != 0 && !isMigratedValueClass(flags))
> 
> do we still need the `isMigratedValueClass` ? Aren't migrated value classes treated the same as ordinary value classes thanks to the work we did to load different classfiles based on preview-ness?

I only adjusted this because this is incorrectly marking Java 25 classes as value classes in InnerClasses. This call didn't affect my patch, should I investigate it now?

> src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java line 1772:
> 
>> 1770:         } else {
>> 1771:             poolbuf.appendChar(target.minorVersion);
>> 1772:             markedPreview = target.minorVersion == ClassFile.PREVIEW_MINOR_VERSION;
> 
> I don't think `target.minorVersion` can ever be preview?

Sure. I thought this path could have been used by -XDforcePreview. Do you think I can just do:


boolean markedPreview = preview.isEnabled() && preview.usesPreview(c.sourcefile);
if (markedPreview) {

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1533#discussion_r2317278677
PR Review Comment: https://git.openjdk.org/valhalla/pull/1533#discussion_r2317276520
PR Review Comment: https://git.openjdk.org/valhalla/pull/1533#discussion_r2317273110
PR Review Comment: https://git.openjdk.org/valhalla/pull/1533#discussion_r2317270357