On Fri, 13 Mar 2026 23:41:19 GMT, Chen Liang <[email protected]> wrote:
>> Currently, the access flags in core reflection are parsed through a central >> factory that dispatches through ReflectionFactory, which examines the >> location plus the class file format version, so that if a Method has STRICT, >> we call AccessFlag.maskToAccessFlag with the right class file format version >> so we do not fail on encountering STRICT. >> >> After recent studies in project Valhalla, we noticed that Hotspot has a >> uniform representation of access flags for all class file versions. This >> means that if we can avoid passing the ClassFileVersion complexities and >> just parse based on the uniform representation Hotspot recognizes. >> >> This change requires moving the AccessFlagSet to jdk.internal so it could be >> accessed by parsers. But we can remove the JavaLangAccess backdoor to fetch >> the class file versions. >> >> Also see the companion Valhalla PR: >> https://github.com/openjdk/valhalla/pull/2209 > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Copyright years Generally, a good refactoring for the modifier/accessflag checks. Tnx src/java.base/share/classes/java/lang/Class.java line 1396: > 1394: // Top-level classes need to report SUPER, using > getClassFileAccessFlags. > 1395: // Module descriptors do not have Class objects so nothing > reports MODULE. > 1396: return (isArray() || getEnclosingClass() != null) Is there a comment somewhere that can be referenced describing the rational for the differences between the modifier bits and the accessFlag bits? src/java.base/share/classes/jdk/internal/reflect/AccessFlagSet.java line 58: > 56: /// If a bit position does not have an access flag defined, its > corresponding > 57: /// array entry is `null`. > 58: public final class AccessFlagSet extends AbstractSet<AccessFlag> { Are there any tests that directly or indirectly test this implementation of Set, in particular the UOE exceptions. src/java.base/share/classes/jdk/internal/reflect/AccessFlagSet.java line 80: > 78: /// The definition may define extraneous flags not present in the > given class file format, so do not derive > 79: /// the mask of defined flags from this definition. > 80: public static AccessFlag[] findDefinition(Location location, > ClassFileFormatVersion cffv) { I presume the cffv parameter is present to align with the Valhalla version. src/java.base/share/classes/jdk/internal/reflect/AccessFlagSet.java line 99: > 97: /// Users no longer have to explicitly assign access flags to their > correct bit positions. > 98: static AccessFlag[] createDefinition(AccessFlag... known) { > 99: var ret = new AccessFlag[Character.SIZE]; Character.SIZE is misleadingly named or used, implying something related Characters. Short.SIZE would be more appropriate and aligns better with the class file ushort in hotspot. ------------- Changes requested by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/30248#pullrequestreview-4009629631 PR Review Comment: https://git.openjdk.org/jdk/pull/30248#discussion_r2990882774 PR Review Comment: https://git.openjdk.org/jdk/pull/30248#discussion_r2990869636 PR Review Comment: https://git.openjdk.org/jdk/pull/30248#discussion_r2990840763 PR Review Comment: https://git.openjdk.org/jdk/pull/30248#discussion_r2990860416
