On Tue, 2 Sep 2025 22:08:24 GMT, Chen Liang <[email protected]> wrote:
>> 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)
Ok, but then shouldn't this add both ACC_IDENTITY and ACC_SUPER and leave it to
the API to decide what to emit based on version?
>> 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?
It's ok to investigate in a separate PR
>> 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) {
IIRC, `forcePreview` makes use of _any_ feature behave as if they were preview
features, meaning you will get classfile version pollution. But it doesn't
alter Target. So yes, the code you suggest should be correct.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1533#discussion_r2318671623
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1533#discussion_r2318672870
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1533#discussion_r2318676190
