On Wed, 15 Apr 2026 14:48:15 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).

I think this is a reasonable approach though I have little doubt something 
somewhere will be tripped up by these changes. There's an existing issue with 
inner classes that we may, or may not want to address (not necessarily in this 
PR).

Thanks

src/hotspot/share/oops/instanceKlass.cpp line 3706:

> 3704: 
> 3705: #if !defined(PRODUCT) || INCLUDE_JVMTI
> 3706: void InstanceKlass::print_class_flags(outputStream* st) const {

This is incomplete. If the class is an inner class then additional access flags 
are possible (private, protected, static).
EDIT: Hmm jvm_constants.h does not recognise this either via 
`JVM_RECOGNIZED_CLASS_MODIFIERS`. Not sure how this should be handled.

src/hotspot/share/oops/method.cpp line 2207:

> 2205:   st->print   (" - constants:         " PTR_FORMAT " ", 
> p2i(constants()));
> 2206:   constants()->print_value_on(st); st->cr();
> 2207:   st->print   (" - access:            0x%x  ", 
> access_flags().as_method_flags()); print_access_flags(st); st->cr();

There is an existing inconsistency here in that we print the raw flags as 
"method flags" only but then we print them all. Your new code implicitly 
filters the flags by only printing the expected method flags.

-------------

PR Review: https://git.openjdk.org/jdk/pull/30746#pullrequestreview-4118484263
PR Review Comment: https://git.openjdk.org/jdk/pull/30746#discussion_r3090932910
PR Review Comment: https://git.openjdk.org/jdk/pull/30746#discussion_r3090984728

Reply via email to