Re: RFR: 8334714: Class-File API leaves preview [v2]

2024-08-26 Thread Chen Liang
On Sat, 24 Aug 2024 13:53:32 GMT, Chen Liang  wrote:

> If Hotspot now parses these files incorrectly, it might be worth to just 
> throw UnsupportedClassVersionError for oak class files?

I was informed in offline discussion that this is unlikely to be changed and 
this change is unlikely to be backported unless for security purposes.

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2310872682


Re: RFR: 8334714: Class-File API leaves preview [v2]

2024-08-24 Thread Chen Liang
On Sat, 24 Aug 2024 05:03:33 GMT, xxDark  wrote:

> If Hotspot now parses these files incorrectly, it might be worth to just 
> throw `UnsupportedClassVersionError` for oak class files?

This would be an important decision; I would wait till Monday when more 
engineers are back to discuss about the histroy of dropping support for such 
files, why this format never made its way to the JVMS, and the correct way to 
handle so as to avoid inconsistency between different versions.

> Edit: I missed the word `preview` in the quote. Does this only apply to 
> *preview* class file features?

Yes, Class-File API is supposed to handle whatever the VM handles. It's 
actually considered a "JVM API" in Oracle's documentations: 
https://docs.oracle.com/en/java/javase/22/vm/class-file-api.html

OpenJDK's preview version policy allows shipping the preview features for 
testing to a wider audience without any implied burden of long-term 
maintenance. So for example, if the Nullable Types JEP preview adds `~` as a 
null marker in `Signature` attribute but changes it to `?` in a subsequent 
preview, the JVM and core libraries will act as if a previous iteration with 
`~` marker never existed; the policy is similar to that for preview libraries.

Unfortunately, there seems to be some misunderstanding around the role of 
preview, so some are asking if preview features that is largely unchanged and 
finalized in a future release can become finalized in an LTS as well. Short 
answer, no: it's an experiment format enabled by the new 6-month release 
schedule.

And note that javap uses ClassFile API, and as a result, javap for release N 
cannot correctly decompile Class Files with version N-1.65535, etc. We are 
planning to decompile on a best-effort basis, that we try to interpret such as 
class as if it's N.65535 (maybe emit a warning), trying to salvage the most out 
of the Class File and printing errors (but continue to output) if we encounter 
anything we don't expect.

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2308402818


Re: RFR: 8334714: Class-File API leaves preview [v2]

2024-08-23 Thread Matt
On Wed, 17 Jul 2024 08:59:07 GMT, Adam Sotona  wrote:

>> Class-File API is leaving preview.
>> This is a removal of all `@PreviewFeature` annotations from Class-File API.
>> It also bumps all `@since` tags and removes 
>> `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains three commits:
> 
>  - Merge branch 'master' into JDK-8334714-final
>  - bumped @since tag
>  - 8334714: Class-File API leaves preview

> Unfortunately, ClassFile API's scope is only to interpret correctly the Class 
> Files that are accepted by the current VM and support writing such class 
> files; for example, **for release N, we will not support correct _parsing_ or 
> writing of preview class files from N-1, N-2, etc.**

I understand that there is no explicit goal for this API to be used for general 
purposes, but it seems really odd that I cannot safely assume that classes that 
are successfully loaded in the current runtime will be parsable. Oak classes 
are a notable exception, and nobody should realistically expect to see an oak 
class ever. However, if we only can guarantee that the current class file 
version will be supported and not even N-1 then no application/library 
expecting reliability _(without constant recompilation to ensure the class 
versions are current)_ should use the API.

Again, its not a goal per-say but it seems really disappointing and odd.

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2308154137


Re: RFR: 8334714: Class-File API leaves preview [v2]

2024-08-23 Thread xxDark
On Sat, 24 Aug 2024 04:56:34 GMT, Chen Liang  wrote:

> 
> Indeed, it would be a good RFE to allow users to override default attribute 
> mappers to parse attributes; this would be extremely useful if users wish to 
> support previous previews that only differed in the attribute formats.
> 
I don't think at this point in time there would be any change in the format of 
any existing attributes, as that would be a binary incompatible change.
But yes, having a way to override parsing of builtin attributes would be nice.

> For the behavioral inconsistencies, we can backport this special handling 
> removal to active jdk update projects or even request specification changes 
> on LTS versions (see https://github.com/openjdk/jdk8u-ri) if this is deemed 
> important enough.

I don't think that many projects use pre-Java 1 format at this point. While I 
saw some libraries that kept their version set to versions like Java 4 for no 
apparent reason... Hey, at least it's not Java 1 version.
If Hotspot now parses these files incorrectly, it might be worth to just throw 
`UnsupportedClassVersionError` for oak class files?

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2308102351


Re: RFR: 8334714: Class-File API leaves preview [v2]

2024-08-23 Thread Chen Liang
On Wed, 17 Jul 2024 08:59:07 GMT, Adam Sotona  wrote:

>> Class-File API is leaving preview.
>> This is a removal of all `@PreviewFeature` annotations from Class-File API.
>> It also bumps all `@since` tags and removes 
>> `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains three commits:
> 
>  - Merge branch 'master' into JDK-8334714-final
>  - bumped @since tag
>  - 8334714: Class-File API leaves preview

I think so. See 
https://mail.openjdk.org/pipermail/hotspot-dev/2019-October/039795.html for 
some context. There are also historical evaluations available at 
https://bugs.openjdk.org/browse/JDK-8232890 and 
https://bugs.openjdk.org/browse/JDK-8232967, notably the judgement in 
Compatibility Risk Description:

> Class files with versions < 45.3 predate Java.

> But, does this mean that now, Hotspot blindly accepts classfiles with version 
> 45 with, technically, incorrect data?

It appears so. See note in `ClassFileFormatVersion`: 
https://github.com/openjdk/jdk/blob/5671f836039ef1683e3e9ce5b7cf0fa2f1860e2d/src/java.base/share/classes/java/lang/reflect/ClassFileFormatVersion.java#L413-L414
It seems that now major version 45, regardless of the minor version, is simply 
seen as the Class File format for Java 1.1.

> Also now, [this](https://github.com/Col-E/CAFED00D) library which took 
> special care to parse oak files reports wrong instructions does not parse 
> classfile at all.

Indeed, it would be a good RFE to allow users to override default attribute 
mappers to parse attributes; this would be extremely useful if users wish to 
support previous previews that only differed in the attribute formats.

> `jdk-1.8.0_402\bin\java -cp . Test`
> `Hello World`

For the behavioral inconsistencies, we can backport this special handling 
removal to active jdk update projects or even request specification changes on 
LTS versions (see https://github.com/openjdk/jdk8u-ri) if this is deemed 
important enough.

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2308099862


Re: RFR: 8334714: Class-File API leaves preview [v2]

2024-08-23 Thread xxDark
On Sat, 24 Aug 2024 03:45:03 GMT, Chen Liang  wrote:

> While your account of oak format seems interesting (from a search, it seems 
> to use u1 for max stacks/locals, u2 for Code size), it is neither recognized 
> by hotspot (the reference implementation for JVM):
> 
> https://github.com/openjdk/jdk/blob/5671f836039ef1683e3e9ce5b7cf0fa2f1860e2d/src/hotspot/share/classfile/classFileParser.cpp#L2332-L2334

Interesting. I guess handling of oak classfiles was removed a long time ago by 
[this](https://github.com/openjdk/jdk17u-dev/commit/eedc99c9ab2647f0233e4899a2976eb6f3095319)
 commit.
But, does this mean that now, Hotspot blindly accepts classfiles with version 
45 with, technically, incorrect data?

java -cp . Test
Hello, World

jdk-1.8.0_402\bin\java -cp . Test
Error: A JNI error has occurred, please check your installation and try again
Exception in thread "main" java.lang.ClassFormatError: Invalid code attribute 
name index 10 in class file Test
at java.lang.ClassLoader.defineClass1(Native Method)
at java.lang.ClassLoader.defineClass(ClassLoader.java:756)

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2308042111


Re: RFR: 8334714: Class-File API leaves preview [v2]

2024-08-23 Thread Chen Liang
On Sat, 24 Aug 2024 03:05:14 GMT, xxDark  wrote:

> Classfiles with oak format seem to be unparsable. They also cannot be 
> written. (maxStack/maxLocals in Code attribute depend on classfile version). 
> Constants such as `JAVA_1_VERSION` exist in `java.lang.classfile.ClassFile`.

Unfortunately, ClassFile API's scope is only to interpret correctly the Class 
Files that are accepted by the current VM and support writing such class files; 
for example, for release N, we will not support correct parsing or writing of 
preview class files from N-1, N-2, etc.

While your account of oak format seems interesting (from a search, it seems to 
use u1 for max stacks/locals, u2 for Code size), it is neither recognized by 
hotspot (the reference implementation for JVM):
https://github.com/openjdk/jdk/blob/5671f836039ef1683e3e9ce5b7cf0fa2f1860e2d/src/hotspot/share/classfile/classFileParser.cpp#L2332-L2334
Nor by the JVMS: 
https://docs.oracle.com/javase/specs/jvms/se22/html/jvms-4.html#jvms-4.7.3

And as such, it falls out of the [goals](https://openjdk.org/jeps/466#Goals) of 
the API, and is likely not going to be added. You most likely will resort to a 
third party library to handle such frozen format, as the main issue ClassFile 
API is solving is that a library built against JDK 21 may not run on JDK 23 
unless it bumps its ASM dependency to support reading JDK 23 Class File format, 
which doesn't exist for the said oak format.

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2308036118


Re: RFR: 8334714: Class-File API leaves preview [v2]

2024-08-23 Thread xxDark
On Wed, 17 Jul 2024 08:59:07 GMT, Adam Sotona  wrote:

>> Class-File API is leaving preview.
>> This is a removal of all `@PreviewFeature` annotations from Class-File API.
>> It also bumps all `@since` tags and removes 
>> `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains three commits:
> 
>  - Merge branch 'master' into JDK-8334714-final
>  - bumped @since tag
>  - 8334714: Class-File API leaves preview

I've been trying to work with classfile api, and:
- `AttributesProcessingOption` is unused, does nothing?
- Classfiles with oak format seem to be unparsable. They also cannot be 
written. (maxStack/maxLocals in Code attribute depend on classfile version). 
Constants such as `JAVA_1_VERSION` exist in `java.lang.classfile.ClassFile`.

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2308012254


Re: RFR: 8334714: Class-File API leaves preview [v2]

2024-07-18 Thread Vicente Romero
On Wed, 17 Jul 2024 08:59:07 GMT, Adam Sotona  wrote:

>> Class-File API is leaving preview.
>> This is a removal of all `@PreviewFeature` annotations from Class-File API.
>> It also bumps all `@since` tags and removes 
>> `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains three commits:
> 
>  - Merge branch 'master' into JDK-8334714-final
>  - bumped @since tag
>  - 8334714: Class-File API leaves preview

lgtm

-

Marked as reviewed by vromero (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19826#pullrequestreview-2186608609


Re: RFR: 8334714: Class-File API leaves preview [v2]

2024-07-17 Thread Adam Sotona
> Class-File API is leaving preview.
> This is a removal of all `@PreviewFeature` annotations from Class-File API.
> It also bumps all `@since` tags and removes 
> `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`.
> 
> Please review.
> 
> Thanks,
> Adam

Adam Sotona has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains three commits:

 - Merge branch 'master' into JDK-8334714-final
 - bumped @since tag
 - 8334714: Class-File API leaves preview

-

Changes: https://git.openjdk.org/jdk/pull/19826/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19826&range=01
  Stats: 715 lines in 166 files changed: 0 ins; 477 del; 238 mod
  Patch: https://git.openjdk.org/jdk/pull/19826.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19826/head:pull/19826

PR: https://git.openjdk.org/jdk/pull/19826


Re: RFR: 8334714: Class-File API leaves preview

2024-07-16 Thread Chen Liang
On Mon, 15 Jul 2024 13:26:39 GMT, ExE Boss  wrote:

>> Class-File API is leaving preview.
>> This is a removal of all `@PreviewFeature` annotations from Class-File API.
>> It also bumps all `@since` tags and removes 
>> `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Note that `Utf8Entry::equalsString` has inconsistent behaviour when called 
> with a `null` value, which I’ve reported as [JI‑9077307], and should probably 
> be fixed before this API leaves preview.
> 
> [JI‑9077307]: https://bugs.openjdk.org/browse/JI-9077307 "JI‑9077307: 
> Inconsistent NPE behaviour of `Utf8Entry::equalsString`"

@ExE-Boss Your report has been promoted to 
https://bugs.openjdk.org/browse/JDK-8336430 and will be addressed in a separate 
patch.

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2232040064


Re: RFR: 8334714: Class-File API leaves preview

2024-07-15 Thread ExE Boss
On Fri, 21 Jun 2024 11:56:37 GMT, Adam Sotona  wrote:

> Class-File API is leaving preview.
> This is a removal of all `@PreviewFeature` annotations from Class-File API.
> It also bumps all `@since` tags and removes 
> `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`.
> 
> Please review.
> 
> Thanks,
> Adam

Note that `Utf8Entry::equalsString` has inconsistent behaviour when called with 
a `null` value, which I’ve reported as [JI‑9077307], and should probably be 
fixed before this API leaves preview.

[JI‑9077307]: https://bugs.openjdk.org/browse/JI-9077307 "JI‑9077307: 
Inconsistent NPE behaviour of `Utf8Entry::equalsString`"

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2228504207


Re: RFR: 8334714: Class-File API leaves preview

2024-06-21 Thread Adam Sotona
On Fri, 21 Jun 2024 12:31:05 GMT, Alan Bateman  wrote:

> Looks like this has been accidentally created as a sub-task of the JEP issue, 
> I assume you'll fix that. Will there be another issue to update the tests and 
> drop `@enablePreview`?

I thought the implementation is usually created as a subtask of JEP, however I 
can make it a standalone bug if neede.

Yes, there will be follow up task to clean all the `@enablePreview` mess, I'll 
create it.

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2182673557


Re: RFR: 8334714: Class-File API leaves preview

2024-06-21 Thread Adam Sotona
On Fri, 21 Jun 2024 11:56:37 GMT, Adam Sotona  wrote:

> Class-File API is leaving preview.
> This is a removal of all `@PreviewFeature` annotations from Class-File API.
> It also bumps all `@since` tags and removes 
> `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`.
> 
> Please review.
> 
> Thanks,
> Adam

Any potential API changes should be merged first, to avoid impression of 
changing a non-preview API.
However any discussion and approval processes of API and documentation changes 
should not block the JEP process (and each other).

Cleaning up preview-enabled tests has a lower priority and should follow.

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-218256


Re: RFR: 8334714: Class-File API leaves preview

2024-06-21 Thread Alan Bateman
On Fri, 21 Jun 2024 11:56:37 GMT, Adam Sotona  wrote:

> Class-File API is leaving preview.
> This is a removal of all `@PreviewFeature` annotations from Class-File API.
> It also bumps all `@since` tags and removes 
> `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`.
> 
> Please review.
> 
> Thanks,
> Adam

Looks like this has been accidentally created as a sub-task of the JEP issue, I 
assume you'll fix that. Will there be another issue to update the tests and 
drop `@enablePreview`?

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2182663458


Re: RFR: 8334714: Class-File API leaves preview

2024-06-21 Thread Chen Liang
On Fri, 21 Jun 2024 11:56:37 GMT, Adam Sotona  wrote:

> Class-File API is leaving preview.
> This is a removal of all `@PreviewFeature` annotations from Class-File API.
> It also bumps all `@since` tags and removes 
> `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`.
> 
> Please review.
> 
> Thanks,
> Adam

Do we intend to do it after we finish our planned API changes or as soon as 
possible? Also we might need to look at the preview-enabled test treatments.

-

PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2182652273