On Sat, 5 Mar 2022 19:54:44 GMT, Joe Darcy <da...@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/f992eace...14980605 > > > The mapping from Location to AccessFlag(s) could be implemented event > > > without using a Map. You just have to be careful not to use EnumSet for > > > that (i.e. before AccessFlag enum constants are fully initialized) - an > > > ArrayList is better for this case anyway. Also, conversion from > > > ModuleDescriptor modifier(s) to AccessFlag(s) could be done without first > > > creating a bitmask and then re-constructing a set of AccessFlag(s) from > > > it. Reflective object modifiers can only be translated via bitmask, but > > > various ModuleDescriptor Modifier(s) may have a direct mapping to > > > corresponding AccessFlag(s). For example: > > > [plevart@5a3cd8f](https://github.com/plevart/jdk/commit/5a3cd8f6851a0deae7e3e5028bba9a51d7863929) > > > > > > Yes, I made the same observation for the module-related modifiers when > > coding this up; I'll look to add some API support to avoid the need to > > detour through bitmasks. > > To get the public API worked out, I wanted to avoid complications in > > inter-dependent class initialization. Thanks for suggesting an alternative; > > I'll consider that when I resume work on this PR (need to start writing > > some tests...) Thanks. > > On a second thought, maybe the common bitmask representation is the way to > go. One one hand it must be computed from the set of module-related Modifier > set(s), but then again such bitmask is very easy to cache economically (a > single int field, no synchronization). When you have a bitmask at hand and a > list of all possible elements (the universe), a very efficient implementation > of a Set is possible (similar to EnumSet). Such AccessFlagSet instances do > not need to be cached since they are just thin views over existing > "permanent" structures and will, as such, typically be optimized away by JIT. > Note the particularly efficient .contains() implementation which just checks > two bitmasks (the flag mask and the location mask). > > [plevart@de9f261](https://github.com/plevart/jdk/commit/de9f2614da56775be4ae07c6781cbec9fbd39930) FWIW, I did link this issue to the RFE [JDK-8145048](https://bugs.openjdk.java.net/browse/JDK-8145048) consider enhancements to EnumMap and EnumSet, which includes a request an immutable EnumSet. ------------- PR: https://git.openjdk.java.net/jdk/pull/7445
