On Fri, 17 Apr 2026 09:37:02 GMT, Casper Norrbin <[email protected]> wrote:
>> Hi everyone, >> >> `AccessFlags::print_on` is used for class, field and method printing, but >> those entities do not share the same modifier set. The current helper >> hard-codes a single mixed list of access flags for all 3 and is thus unaware >> of the type of the printed value. Instead of using a single shared printing >> function, we should move the printing to the relevant class, field, and >> method call sites. This makes the printer aware of the type it is printing >> and lets us check only the flags that are relevant for that type. >> >> For this change, I remove `AccessFlags::print_on` and split the printing >> into 3 separate helpers: >> - `InstanceKlass::print_class_flags` >> - `Method::print_access_flags` >> - `fieldDescriptor::print_access_flags` >> >> As a part of that, I added the missing `AccessFlags` predicates used by each >> of the new printers, and updated each printer to check all the flags >> relevant for its type, as defined by `jvm_constants.h`. This lets us cover >> class-specific, method-specific, and field-specific modifiers that were not >> handled before. >> >> I also added new gtests covering each of the 3 printing helpers and the >> previously missing flags. >> >> Testing: >> - Oracle tiers 1-3 >> - New gtests covering each of the printing helpers and each of the new flags >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Casper Norrbin has updated the pull request incrementally with one additional > commit since the last revision: > > reorder private/public/protected in instanceklass Looks good besides an unrelated issue not for this patch. src/hotspot/share/oops/instanceKlass.cpp line 3716: > 3714: if (flags.is_annotation()) st->print("annotation "); > 3715: if (flags.is_enum ()) st->print("enum "); > 3716: if (flags.is_synthetic ()) st->print("synthetic "); The bit for SYNTHETIC, 0x1000, comes before ANNOTATION, 0x2000, and ENUM, 0x4000. That should be tracked in a separate issue though. src/hotspot/share/runtime/fieldDescriptor.cpp line 121: > 119: if (flags.is_transient()) st->print("transient "); > 120: if (flags.is_enum ()) st->print("enum "); > 121: if (flags.is_synthetic()) st->print("synthetic "); Same SYNTHETIC remark here. ------------- Marked as reviewed by liach (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/30746#pullrequestreview-4177512518 PR Review Comment: https://git.openjdk.org/jdk/pull/30746#discussion_r3144228493 PR Review Comment: https://git.openjdk.org/jdk/pull/30746#discussion_r3144230185
