RFR: JDK-8266670: Better modeling of access flags in core reflection [v17]
liach
duke at openjdk.java.net
Wed Apr 13 21:25:20 UTC 2022
On Wed, 13 Apr 2022 21:12:40 GMT, ExE Boss <duke at openjdk.java.net> wrote:
>> 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/afd02683...14980605
>
> src/java.base/share/classes/java/lang/module/ModuleDescriptor.java line 167:
>
>> 165: * but is optional in the dynamic phase, during execution.
>> 166: */
>> 167: STATIC(AccessFlag.STATIC.mask()),
>
> This is actually `AccessFlag.STATIC_PHASE` (`0x0040`), and not `AccessFlag.STATIC` (`0x0008`):
> Suggestion:
>
> STATIC(AccessFlag.STATIC_PHASE.mask()),
> In the current hodgepodge AccessFlag, we have STATIC and STATIC_PHASE, and the incorrect ModuleDescriptor.accessFlags().contains(AccessFlag.STATIC) call is much more subtle, especially to new users of this class. Arguably, this misuse would be way worse than that in the distinct enum case.
Oops, didn't know this already happened. Good spot right there.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7445
More information about the core-libs-dev
mailing list