8347063: Add comments in ClassFileFormatVersion for class file format evolution history #22934

liach · 2025-01-06T18:26:54Z

javax.lang.model.SourceVersion has a series of comments describing the new language features present in each source version. Similar comments for the ClassFileFormatVersion would be helpful, so readers no longer need to search through the JVMS to find changes in new versions.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 2 Reviewers)

Issue

JDK-8347063: Add comments in ClassFileFormatVersion for class file format evolution history (Enhancement - P4)

Reviewers

Joe Darcy (@jddarcy - Reviewer)
Iris Clark (@irisclark - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/22934/head:pull/22934
$ git checkout pull/22934

Update a local copy of the PR:
$ git checkout pull/22934
$ git pull https://git.openjdk.org/jdk.git pull/22934/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22934

View PR using the GUI difftool:
$ git pr show -t 22934

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22934.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-01-06T18:27:56Z

👋 Welcome back liach! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-01-06T18:28:43Z

@liach This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8347063: Add comments in ClassFileFormatVersion for class file format evolution history

Reviewed-by: darcy, iris

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 91 new commits pushed to the master branch:

10f7142: 8347295: Fix WinResourceTest to make it work with WiX v4.0+
01c8ba2: 8347298: Bug in JPackageCommand.ignoreFakeRuntime()
d69463e: 8347300: Don't exclude the "PATH" var from the environment when running app launchers in jpackage tests
4b554b5: 8346722: (fs) Files.probeContentType throws ClassCastException with custom file system provider
46ba515: 8346669: Increase abstraction in SetupBuildLauncher and remove extra args
e7e8f60: 8347302: Mark test tools/jimage/JImageToolTest.java as flagless
9cf7d42: 8346184: C2: assert(has_node(i)) failed during split thru phi
1bf2f5c: 8343510: JFR: Remove AccessControlContext from FlightRecorder::addListener specification
beb0e60: 8347397: Cleanup of JDK-8169880
c5c4efd: 8347120: Launchers should not have java headers on include path
... and 81 more: https://git.openjdk.org/jdk/compare/8d388ccd9eae279018bf499e49152fd276aab5bf...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-01-06T18:29:14Z

@liach The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-01-06T18:31:46Z

Webrevs

jddarcy · 2025-01-06T19:44:36Z

/reviewers 2 reviewer

jddarcy

Generally looks fine and the changes per release look plausible; I didn't double-check them.

openjdk · 2025-01-06T19:45:59Z

@jddarcy
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 2 Reviewers).

irisclark

Verified via consultation of the various platform JSRs.

Though this isn't done in SourceVersion, I'm wondering if it would be helpful to reference the associated JEPs which describe these changes (e.g. JEP 12 for Preview Features, 359 for Records (Preview), 384 Records (Second Preview), 395 Records, etc.).

irisclark · 2025-01-06T20:16:52Z

src/java.base/share/classes/java/lang/reflect/ClassFileFormatVersion.java

+     *  8: private, static, and non-abstract (default) methods in interfaces;
+     *     type annotations (Runtime(Inv/V)isibleTypeAnnotations);
+     *     MethodParameters
+     *  9: modules (Module, ModuleMainClass, ModulePackages, CONSTANT_Module,


Consider referencing JSR 376 for the introduction of Modules. JEP 261 describes the changes to class files.

liach · 2025-01-06T21:22:56Z

I have added the JEP and JSR numbers for some of the features, and clarified if the changed features are attributes, modifiers, constant pool entries, or instructions/opcodes.

irisclark

Looks good. Note that changes in JDK 7 and earlier pre-date JEPs, so no reference of that type is possible.

dholmes-ora · 2025-01-07T04:30:50Z

src/java.base/share/classes/java/lang/reflect/ClassFileFormatVersion.java

+     *  1: InnerClasses, Synthetic, Deprecated attributes
+     *  2: ACC_STRICT modifier
+     *  3: no changes
+     *  4: no changes


The version mapping can be tricky here: is "1" 1.0 plus 1.1? I presume "2" is 1.2, "3" is 1.3.x, "4" is 1.4.x

Might be useful to include the actual JVMS classfile version numbers for ease of reference back to JVMS.

I assume the wording implicitly is referring to the enum position, which uses a "RELEASE_$N" convention. However, I agree that adding the major version in some form would aid people more familiar with those numbers. One possibility:

3 (47.0) no changes 4 (48.0) no changes

If that is adopted, perhaps the preview features could be listed with the minor version set.

How about versions like 1.1, ... 1.8, 9, ... which follows the since versions of libraries and should have no ambiguity? I will commit if everyone agrees.

liach · 2025-01-08T15:23:23Z

Joe and David, can you look at this updated versioning that uses the core libraries since scheme?

irisclark

Versions matching java.specification.version look good.

dholmes-ora · 2025-01-09T03:29:21Z

Joe and David, can you look at this updated versioning that uses the core libraries since scheme?

Yep that looks fine. Thanks.

jddarcy · 2025-01-10T21:02:16Z

Joe and David, can you look at this updated versioning that uses the core libraries since scheme?

Yep that looks fine. Thanks.

I think it is fine it its current form, but would be better also listing the class file format version:

1.1/45.3
1.2/46.0

etc. However, I'll approve the PR in it current state.

liach · 2025-01-10T21:22:31Z

Technically this 45.0 vs 45.3 only matters for 1.0 vs 1.1; those are intricate details like "Oak classes" with u1 max stacks and locals which was removed from hotspot around JDK 14 or so and never part of the JVMS. Now we really only care about majors, and we have the majors in the enums themselves, so I wasn't too eager to jam up the list of versions with the major versions.

Thanks for the reviews.

/integrate

openjdk · 2025-01-10T21:23:51Z

Going to push as commit 6f1f2f2.
Since your change was applied there have been 91 commits pushed to the master branch:

10f7142: 8347295: Fix WinResourceTest to make it work with WiX v4.0+
01c8ba2: 8347298: Bug in JPackageCommand.ignoreFakeRuntime()
d69463e: 8347300: Don't exclude the "PATH" var from the environment when running app launchers in jpackage tests
4b554b5: 8346722: (fs) Files.probeContentType throws ClassCastException with custom file system provider
46ba515: 8346669: Increase abstraction in SetupBuildLauncher and remove extra args
e7e8f60: 8347302: Mark test tools/jimage/JImageToolTest.java as flagless
9cf7d42: 8346184: C2: assert(has_node(i)) failed during split thru phi
1bf2f5c: 8343510: JFR: Remove AccessControlContext from FlightRecorder::addListener specification
beb0e60: 8347397: Cleanup of JDK-8169880
c5c4efd: 8347120: Launchers should not have java headers on include path
... and 81 more: https://git.openjdk.org/jdk/compare/8d388ccd9eae279018bf499e49152fd276aab5bf...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-01-10T21:23:57Z

@liach Pushed as commit 6f1f2f2.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

CFFV evolution comments

77a723b

openjdk bot added the rfr Pull request is ready for review label Jan 6, 2025

openjdk bot added the core-libs [email protected] label Jan 6, 2025

jddarcy approved these changes Jan 6, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 6, 2025

openjdk bot removed the ready Pull request is ready to be integrated label Jan 6, 2025

irisclark approved these changes Jan 6, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 6, 2025

Add JEP/JSR numbers, clarify if something is attribute/modifier/etc.

1339629

openjdk bot removed the ready Pull request is ready to be integrated label Jan 6, 2025

liach requested review from jddarcy and irisclark January 6, 2025 21:23

irisclark approved these changes Jan 6, 2025

View reviewed changes

dholmes-ora reviewed Jan 7, 2025

View reviewed changes

Use since-style versions

f34c91c

irisclark approved these changes Jan 8, 2025

View reviewed changes

jddarcy approved these changes Jan 10, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 10, 2025

openjdk bot added the integrated Pull request has been integrated label Jan 10, 2025

openjdk bot closed this Jan 10, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8347063: Add comments in ClassFileFormatVersion for class file format evolution history #22934

8347063: Add comments in ClassFileFormatVersion for class file format evolution history #22934

liach commented Jan 6, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jan 6, 2025

openjdk bot commented Jan 6, 2025 •

edited

Loading

openjdk bot commented Jan 6, 2025

mlbridge bot commented Jan 6, 2025 •

edited

Loading

jddarcy commented Jan 6, 2025

jddarcy left a comment

openjdk bot commented Jan 6, 2025

irisclark left a comment

irisclark Jan 6, 2025

liach commented Jan 6, 2025

irisclark left a comment

dholmes-ora Jan 7, 2025 •

edited

Loading

jddarcy Jan 7, 2025

liach Jan 7, 2025

liach commented Jan 8, 2025

irisclark left a comment

dholmes-ora commented Jan 9, 2025

jddarcy commented Jan 10, 2025

liach commented Jan 10, 2025

openjdk bot commented Jan 10, 2025

openjdk bot commented Jan 10, 2025

8347063: Add comments in ClassFileFormatVersion for class file format evolution history #22934

8347063: Add comments in ClassFileFormatVersion for class file format evolution history #22934

Conversation

liach commented Jan 6, 2025 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jan 6, 2025

openjdk bot commented Jan 6, 2025 • edited Loading

openjdk bot commented Jan 6, 2025

mlbridge bot commented Jan 6, 2025 • edited Loading

Webrevs

jddarcy commented Jan 6, 2025

jddarcy left a comment

Choose a reason for hiding this comment

openjdk bot commented Jan 6, 2025

irisclark left a comment

Choose a reason for hiding this comment

irisclark Jan 6, 2025

Choose a reason for hiding this comment

liach commented Jan 6, 2025

irisclark left a comment

Choose a reason for hiding this comment

dholmes-ora Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

jddarcy Jan 7, 2025

Choose a reason for hiding this comment

liach Jan 7, 2025

Choose a reason for hiding this comment

liach commented Jan 8, 2025

irisclark left a comment

Choose a reason for hiding this comment

dholmes-ora commented Jan 9, 2025

jddarcy commented Jan 10, 2025

liach commented Jan 10, 2025

openjdk bot commented Jan 10, 2025

openjdk bot commented Jan 10, 2025

liach commented Jan 6, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jan 6, 2025 •

edited

Loading

mlbridge bot commented Jan 6, 2025 •

edited

Loading

dholmes-ora Jan 7, 2025 •

edited

Loading