On Wed, 28 Jan 2026 14:03:58 GMT, Casper Norrbin <[email protected]> wrote:

> Hi everyone,
> 
> `test_classPrinter` tests `classPrinter` using regex to verify that the 
> output matches the expected format.  
> The `print_classes` test includes (among other things) printing the class 
> details for `java.lang.Integer`. 
> 
> On mainline, the static fields are printed like this:
> 
> Java mirror oop for java/lang/Integer: java.lang.Class 
> {0x0000000efc0427a0} - klass: 'java/lang/Class' - flags: 
>  - ---- fields (total size 19 words):
> ...
> - ---- static fields (2):
>  - public static final 'MIN_VALUE' 'I' @136  -2147483648 (0x80000000)
>  - public static final 'MAX_VALUE' 'I' @140  2147483647 (0x7fffffff)
> ...
> 
> 
> With the Valhalla changes, the output now includes `value`:
> 
> Java mirror oop for java/lang/Integer: java.lang.Class 
> {0x000000069f040728} - klass: 'java/lang/Class' - flags: 
>  - ---- fields (total size 19 words):
> ...
>  - ---- static fields (2):
>  - public static final value 'MIN_VALUE' 'I' @136  -2147483648 (0x80000000)
>  - public static final value 'MAX_VALUE' 'I' @140  2147483647 (0x7fffffff)
> ...
> 
> 
> When printing fields' access flags, we now also print `identity`/`value` 
> depending on whether a field has identity. Most fields (including 
> `MIN_VALUE`) do not have identity and therefore show `value`. The existing 
> regex did not account for this new print, making the test fail. I have 
> updated it accordingly and the test now passes again.
> 
> Testing:
> - Tier 1

Having thought about it a bit more, I think I'd prefer if we guard the printing 
of identity/value behind a feature flag, given that JVMTI has access to this 
information too. For example updating accessFlags.cpp:

  if (Arguments::is_valhalla_enabled()) {
    if (is_identity_class()) st->print("identity "  );
    if (!is_identity_class()) st->print("value "    );
  }

And of course, the test would have to be updated accordingly.

This is not a thing I'd die on a hill for though, so if other folks want to 
move forward as-is, I'd be okay with it as well.

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

PR Comment: https://git.openjdk.org/valhalla/pull/1989#issuecomment-3816307388

Reply via email to