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

[Joe Darcy]

On Thu, 17 Feb 2022 20:32:45 GMT, ExE Boss  wrote:

> Note that the `accessFlags()` method is still missing in the 
> `ModuleDescriptor.Opens` and the top‑level `ModuleDescriptor` classes:
> 
> https://github.com/openjdk/jdk/blob/5cc6ba50b1155340d13bc29d581c91ac543d2d95/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java#L637-L651
> 
> https://github.com/openjdk/jdk/blob/5cc6ba50b1155340d13bc29d581c91ac543d2d95/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java#L1307-L1324

You're correct; addressed in subsequent push. Thanks.

-

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

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

[ExE Boss]

On Thu, 17 Feb 2022 19:26:13 GMT, Joe Darcy  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 11 additional commits since 
> the last revision:
> 
>  - Add support for module flags; fix typo.
>  - Merge branch 'master' into JDK-8266670
>  - Update JVMS references.
>  - Merge branch 'master' into JDK-8266670
>  - Reorder constants by mask value per review feedback.
>  - Merge branch 'master' into JDK-8266670
>  - Respond to more review feedback.
>  - Respond to review feedback explicitly stating returned sets are immutable.
>  - Fix typo from review feedback.
>  - Merge branch 'master' into JDK-8266670
>  - ... and 1 more: 
> https://git.openjdk.java.net/jdk/compare/b1511105...5cc6ba50

Note that the `accessFlags()` method is still missing in the 
`ModuleDescriptor.Opens` and the top‑level `ModuleDescriptor` classes: 

 


-

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

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

[Joe Darcy]

> 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 11 additional commits since the 
last revision:

 - Add support for module flags; fix typo.
 - Merge branch 'master' into JDK-8266670
 - Update JVMS references.
 - Merge branch 'master' into JDK-8266670
 - Reorder constants by mask value per review feedback.
 - Merge branch 'master' into JDK-8266670
 - Respond to more review feedback.
 - Respond to review feedback explicitly stating returned sets are immutable.
 - Fix typo from review feedback.
 - Merge branch 'master' into JDK-8266670
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/d7b3d5dd...5cc6ba50

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7445/files
  - new: https://git.openjdk.java.net/jdk/pull/7445/files/02228108..5cc6ba50

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7445=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7445=05-06

  Stats: 494 lines in 13 files changed: 48 ins; 412 del; 34 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7445.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7445/head:pull/7445

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