On Wed, 15 Apr 2026 15:45:26 GMT, Chen Liang <[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). > > 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. I am surprised that we only ever print access flags in relation to JVMTI - I would have expected logging or crash reporting to do so. In any case I also find the `!PRODUCT` a little jarring. Given we always have JVMTI it makes no different to our binaries. > 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. +1 > 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. This will be short-lived either way as valhalla gets rid of the legacy strictfp notion for methods anyway. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30746#discussion_r3090996397 PR Review Comment: https://git.openjdk.org/jdk/pull/30746#discussion_r3090998411 PR Review Comment: https://git.openjdk.org/jdk/pull/30746#discussion_r3091021640
