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).

This direction looks reasonable. Similar effort happens in core libraries in 
#30093.

src/hotspot/share/runtime/fieldDescriptor.hpp line 114:

> 112:   void print_on(outputStream* st) const;
> 113:   void print_on_for(outputStream* st, oop obj);
> 114: #if !defined(PRODUCT) || INCLUDE_JVMTI

The new `!defined(PRODUCT)` seems to be only for the gtest. I wonder if gtest 
can run the test for `print_access_flags` based on `INCLUDE_JVMTI` flag 
instead; the macro condition tweak seems weird.

src/hotspot/share/utilities/accessFlags.hpp line 58:

> 56:   bool is_bridge      () const         { return (_flags & JVM_ACC_BRIDGE  
>     ) != 0; }
> 57:   bool is_transient   () const         { return (_flags & 
> JVM_ACC_TRANSIENT   ) != 0; }
> 58:   bool has_vararg     () const         { return (_flags & JVM_ACC_VARARGS 
>     ) != 0; }

I recommend `is_varargs`, this flag is not enforced by the VM so technically 
users can create class files that declare varargs methods without a trailing 
array argument.

src/hotspot/share/utilities/accessFlags.hpp line 64:

> 62:   bool is_interface   () const         { return (_flags & 
> JVM_ACC_INTERFACE   ) != 0; }
> 63:   bool is_abstract    () const         { return (_flags & 
> JVM_ACC_ABSTRACT    ) != 0; }
> 64:   bool is_strict_method() const        { return (_flags & JVM_ACC_STRICT  
>     ) != 0; }

If "strict method" sounds too weird, we can call this "strictfp" following the 
original Java language modifier's name.

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

PR Review: https://git.openjdk.org/jdk/pull/30746#pullrequestreview-4114905404
PR Review Comment: https://git.openjdk.org/jdk/pull/30746#discussion_r3087683294
PR Review Comment: https://git.openjdk.org/jdk/pull/30746#discussion_r3087689517
PR Review Comment: https://git.openjdk.org/jdk/pull/30746#discussion_r3087658874

Reply via email to