On Wed, 25 Mar 2026 20:32:13 GMT, Roger Riggs <[email protected]> wrote:

>> Chen Liang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Copyright years
>
> src/java.base/share/classes/java/lang/Class.java line 1396:
> 
>> 1394:         // Top-level classes need to report SUPER, using 
>> getClassFileAccessFlags.
>> 1395:         // Module descriptors do not have Class objects so nothing 
>> reports MODULE.
>> 1396:         return (isArray() || getEnclosingClass() != null)
> 
> Is there a comment somewhere that can be referenced describing the rational 
> for the differences between the modifier bits and the accessFlag bits?

I was only aware of this excerpt below, aside from the "SUPER" bit status. I am 
not aware of the reason we hid the SUPER bit from `getModifiers` but not other 
bits without source modifier correspondence since 1.5.

https://github.com/openjdk/jdk/blob/ee3665bca026fe53409df8391d49477c64ae23a2/src/java.base/share/classes/jdk/internal/reflect/Reflection.java#L76-L84

> src/java.base/share/classes/jdk/internal/reflect/AccessFlagSet.java line 58:
> 
>> 56: /// If a bit position does not have an access flag defined, its 
>> corresponding
>> 57: /// array entry is `null`.
>> 58: public final class AccessFlagSet extends AbstractSet<AccessFlag> {
> 
> Are there any tests that directly or indirectly test this implementation of 
> Set, in particular the UOE exceptions.

They are currently indirectly tested here:
https://github.com/openjdk/jdk/blob/aeea8497562aabda12f292ad93c9f0f6935cc842/test/jdk/java/util/Collection/MOAT.java#L142-L143

> src/java.base/share/classes/jdk/internal/reflect/AccessFlagSet.java line 80:
> 
>> 78:     /// The definition may define extraneous flags not present in the 
>> given class file format, so do not derive
>> 79:     /// the mask of defined flags from this definition.
>> 80:     public static AccessFlag[] findDefinition(Location location, 
>> ClassFileFormatVersion cffv) {
> 
> I presume the cffv parameter is present to align with the Valhalla version.

Yes: 
https://github.com/openjdk/valhalla/blob/5b0e66af983a9e9b7291ded9e7d7dbcde0f41d0e/src/java.base/share/classes/jdk/internal/reflect/AccessFlagSet.java#L80

> src/java.base/share/classes/jdk/internal/reflect/AccessFlagSet.java line 99:
> 
>> 97:     /// Users no longer have to explicitly assign access flags to their 
>> correct bit positions.
>> 98:     static AccessFlag[] createDefinition(AccessFlag... known) {
>> 99:         var ret = new AccessFlag[Character.SIZE];
> 
> Character.SIZE is misleadingly named or used, implying something related 
> Characters.
> Short.SIZE would be more appropriate and aligns better with the class file 
> ushort in hotspot.

This is somewhat an open question - char is equivalent to u2; Class uses char 
to represent modifiers and classFileAccessFlags, which was presumably due to 
its zero-extension behavior. I want to hear more voices on this topic.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30248#discussion_r2990947587
PR Review Comment: https://git.openjdk.org/jdk/pull/30248#discussion_r2990937493
PR Review Comment: https://git.openjdk.org/jdk/pull/30248#discussion_r2990922817
PR Review Comment: https://git.openjdk.org/jdk/pull/30248#discussion_r2990929501

Reply via email to