RFR: 8347471: Provide valid flags and mask in AccessFlag.Location [v8]

Roger Riggs rriggs at openjdk.org
Mon Apr 28 20:04:48 UTC 2025


On Sat, 26 Apr 2025 19:44:02 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Some AccessFlag parsing methods throw IAE because a flag mask is not valid in a location. However, there is no easy way to check what flag mask bits or what flags are valid for a location. We need such APIs to check, specific to each class file format version.
>> 
>> Also in the investigation, it's noted that `ACC_SYNTHETIC` is incorrectly represented - it is available since release 5.0 instead of release 7. This bug is fixed together for implementation simplicity.
>> 
>> The new methods are all in `AccessFlag.Location`:
>> - `Set<AccessFlag> flags()`
>> - `int flagsMask()`
>> - `Set<AccessFlag> flags(ClassFileFormatVersion)`
>> - `int flagsMask(ClassFileFormatVersion)`
>> 
>> Also there is some simplification to `AccessFlag` itself to remove the anonymous classes, which should be more startup-friendly.
>> 
>> Testing: Tier 1-3
>
> Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix iterator missing NSEE

src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 345:

> 343:      * <p>
> 344:      * This method may return an empty set if the flag is not defined in
> 345:      * the current class file format version.

"may" is conditional, but the empty set is always returned when the flag is not specified.
Suggestion:

     * This method returns an empty set if the flag is not defined in
     * the current class file format version.

src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 356:

> 354:      * <p>
> 355:      * This method may return an empty set if the flag is not defined in
> 356:      * the given {@code cffv}.

Suggestion:

     * This method returns an empty set if the flag is not defined in
     * the given {@code cffv}.

src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 634:

> 632:          * <p>
> 633:          * This method may return {@code 0} if the structure does not exist in
> 634:          * the given {@code cffv}.

Suggestion:

         * {@return the union of masks of all access flags defined for
         * this location in the given class file format version}
         * <p>
         * This method returns {@code 0} if there are no access flags in
         * the given {@code cffv}.

The word structure seems out of place and indefinite.

src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 649:

> 647:          * <p>
> 648:          * This method may return an empty set if the structure does not exist
> 649:          * in the current class file format version.

Suggestion:

         * {@return The set of {@code AccessFlags} defined for this location in the current class file format version}
         * <p>
         * This method returns an empty set if the structure does not exist
         * in the current class file format version.

src/java.base/share/classes/java/lang/reflect/AccessFlag.java line 662:

> 660:          * <p>
> 661:          * This method may return an empty set if the structure does not exist
> 662:          * in the given {@code cffv}.

Suggestion:

         * {@return The set of  {@code AccessFlags} defined for this location in the given class file format version}
         * <p>
         * This method returns an empty set if the structure does not exist
         * in the given {@code cffv}.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23095#discussion_r2064414287
PR Review Comment: https://git.openjdk.org/jdk/pull/23095#discussion_r2064408776
PR Review Comment: https://git.openjdk.org/jdk/pull/23095#discussion_r2064445683
PR Review Comment: https://git.openjdk.org/jdk/pull/23095#discussion_r2064448181
PR Review Comment: https://git.openjdk.org/jdk/pull/23095#discussion_r2064450751


More information about the core-libs-dev mailing list