RFR: JDK-8266670: Better modeling of access flags in core reflection [v17]

[Mandy Chung]

On Sat, 5 Mar 2022 19:54:44 GMT, Joe Darcy <darcy at openjdk.org> wrote:

>> This is an early review of changes to better model JVM access flags, that is "modifiers" like public, protected, etc. but explicitly at a VM level.
>> 
>> Language level modifiers and JVM level access flags are closely related, but distinct. There are concepts that overlap in the two domains (public, private, etc.), others that only have a language-level modifier (sealed), and still others that only have an access flag (synthetic).
>> 
>> The existing java.lang.reflect.Modifier class is inadequate to model these subtleties. For example, the bit positions used by access flags on different kinds of elements overlap (such as "volatile" for fields and "bridge" for methods. Just having a raw integer does not provide sufficient context to decode the corresponding language-level string. Methods like Modifier.methodModifiers() were introduced to cope with this situation.
>> 
>> With additional modifiers and flags on the horizon with projects like Valhalla, addressing the existent modeling deficiency now ahead of time is reasonable before further strain is introduced.
>> 
>> This PR in its current form is meant to give the overall shape of the API. It is missing implementations to map from, say, method modifiers to access flags, taking into account overlaps in bit positions.
>> 
>> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in once the API is further along.
>
> Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 26 additional commits since the last revision:
> 
>  - Respond to review feedback.
>  - Merge branch 'master' into JDK-8266670
>  - Make workding changes suggested in review feedback.
>  - Merge branch 'master' into JDK-8266670
>  - Typo fix; add implSpec to Executable.
>  - Appease jcheck.
>  - Fix some bugs found by inspection, docs cleanup.
>  - Merge branch 'master' into JDK-8266670
>  - Initial support for accessFlags methods
>  - Add mask to access flag functionality.
>  - ... and 16 more: https://git.openjdk.java.net/jdk/compare/7ac698ba...14980605

I took a closer look at the proposed APIs.   I think it's in a good state to target for 19.   I skimmed on the existing JDK usage of `getModifiers` other than a trivial test e.g. is public, final, etc and the proposed API works well (btw no plan to convert them and just use those cases for validation). 

The value of `ACC_SUPER` and `ACC_STRICT` could possibly be reused for new access flags.   It may be useful to document when the flag becomes obsolete.

Nit: the enum constants are listed in the order of the mask value, which I like.   Some enum constants reference the `Modifer` constants but I think it'd help to see the mask value here consistently for all entries.   One go-to place in the source if I want to find the value of different flags.

src/java.base/share/classes/java/lang/Class.java line 1328:

> 1326:      * @since 19
> 1327:      */
> 1328:     public Set<AccessFlag> accessFlags() {

The access flags of a hidden class has no difference than that of a normal class.  A hidden class is created with normal `ClassFile` except that it's created via `Lookup::defineHiddenClass` API. 

I think the `Class::accessFlags` method should specify the access flags for primitive type, void, and array classes as `Class::getModifiers` specified.

src/java.base/share/classes/java/lang/module/ModuleDescriptor.java line 216:

> 214: 
> 215:         /**
> 216:          * {@return an unmodifiable set of the module {@linkplain AccessFlag

Suggestion:

         * {@return a possibly-empty unmodifiable set of the module {@linkplain AccessFlag


as specified in the `@return` of the `modifiers()` method.   Same comment applies to the `accessFlags` method in `ModuleDescriptor.Opens` and other classes.

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

> 42:  * <em>not</em> have an access flag, such as {@code sealed} (JVMS
> 43:  * {@jvms 4.7.31}) and some access flags have no corresponding
> 44:  * modifier, such as {@linkplain SYNTHETIC synthetic}.

Suggestion:

 * modifier, such as {@linkplain #SYNTHETIC synthetic}.

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

> 184:     /**
> 185:      * The access flag {@code ACC_ABSTRACT}, corresponding to the
> 186:      * source modifier {@code link Modifier#ABSTRACT abstract}.

Suggestion:

     * source modifier {@link Modifier#ABSTRACT abstract}.

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

> 304:      * Note that since these locations represent class file structures
> 305:      * rather than language structures many language structures, such
> 306:      * as constructors and interfaces, are <em>not</em> present.

missing `@since 19`

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

PR: https://git.openjdk.java.net/jdk/pull/7445