On Sat, 26 Apr 2025 19:44:02 GMT, Chen Liang <li...@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
