Skip to content

8355652: Parse ClassFileFormatVersion from ClassFileVersion #26406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+18 −1
Open

8355652: Parse ClassFileFormatVersion from ClassFileVersion #26406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ab67e49
8355652: add new method to return ClassFileFormatVersion from ClassFi…
gustavosimon Jul 20, 2025
7134042
8355652: update copyright year to 2025 in ClassFileVersionImpl.java
gustavosimon Jul 21, 2025
d4ff0b6
8355652: update copyright year to 2025 in ClassFileVersionImpl.java
gustavosimon Jul 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/java.base/share/classes/java/lang/classfile/ClassFileVersion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
package java.lang.classfile;

import java.lang.reflect.ClassFileFormatVersion;
import java.util.Optional;

import jdk.internal.classfile.impl.ClassFileVersionImpl;

Expand Down Expand Up @@ -76,6 +77,11 @@ public sealed interface ClassFileVersion
*/
int minorVersion();

/**
* {@return a {@link ClassFileFormatVersion} element}.
*/
Optional<ClassFileFormatVersion> formatVersion();

/**
* {@return a {@link ClassFileVersion} element}
* @param majorVersion the major version
Expand Down
13 changes: 12 additions & 1 deletion src/java.base/share/classes/jdk/internal/classfile/impl/ClassFileVersionImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -25,6 +25,8 @@
package jdk.internal.classfile.impl;

import java.lang.classfile.ClassFileVersion;
import java.lang.reflect.ClassFileFormatVersion;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: copyright year :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@myankelev I updated the copyright year in ClassFileVersionImpl

import java.util.Optional;

public final class ClassFileVersionImpl
extends AbstractElement
Expand All @@ -46,6 +48,15 @@ public int minorVersion() {
return minorVersion;
}

@Override
public Optional<ClassFileFormatVersion> formatVersion() {
try {
return Optional.of(ClassFileFormatVersion.fromMajor(majorVersion));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check the minor version to be 0 for major versions 54 and later.

Copy link
Contributor Author

@gustavosimon gustavosimon Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean something like this?

@Override
    public Optional<ClassFileFormatVersion> formatVersion() {
        if (majorVersion < 54 || minorVersion != 0) {
            return Optional.empty();
        }
        try {
            return Optional.of(ClassFileFormatVersion.fromMajor(majorVersion));
        } catch (IllegalArgumentException e) {
            return Optional.empty();
        }
    }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this logic is wrong. It's more like the check in

jdk/src/java.base/share/classes/jdk/internal/misc/VM.java

Lines 174 to 180 in 441dbde

public static boolean isSupportedClassFileVersion(int major, int minor) {
if (major < ClassFile.JAVA_1_VERSION || major > ClassFile.latestMajorVersion()) return false;
// for major version is between 45 and 55 inclusive, the minor version may be any value
if (major < ClassFile.JAVA_12_VERSION) return true;
// otherwise, the minor version must be 0 or 65535
return minor == 0 || (minor == ClassFile.PREVIEW_MINOR_VERSION && major == ClassFile.latestMajorVersion());
} 

 public Optional<ClassFileFormatVersion> formatVersion() { 
     if (majorVersion < ClassFile.JAVA_1_VERSION || majorVersion > ClassFile.latestMajorVersion()) return Optional.empty(); 
     // for major version is between 45 and 55 inclusive, the minor version may be any value 
     if (majorVersion >= ClassFile.JAVA_12_VERSION && minorVersion != 0) return Optional.empty(); 
     // otherwise, only minor version 0 is a defined, persistent format
     return Optional.of(ClassFileFormatVersion.fromMajor(majorVersion)); 
 }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Could you have this hook in a private class method or a static public method?
Maybe it can be reused in another points?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it can be reused in another points?

ClassFileFormatVersion and Optional are more user-oriented classes; internal code don't really ever use these two types, so I don't think you need to extract this logic specifically.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean about the rule the version - similar to isSupportedClassFileVersion method

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't think so. I intentionally proposed the original RFE so we have one place where we can consolidate this handling, and that place should be here instead of some other place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I'll do this way

} catch (IllegalArgumentException e) {
return Optional.empty();
}
}

@Override
public void writeTo(DirectClassBuilder builder) {
builder.setVersion(majorVersion, minorVersion);
Expand Down