On Thu, 10 Sep 2026 08:27:54 GMT, Stefan Karlsson <[email protected]> wrote:
>> src/hotspot/share/runtime/fieldDescriptor.cpp line 314:
>>
>>> 312: if (obj == nullptr) {
>>> 313: assert(vpc == nullptr, "flattening not supported for static
>>> fields");
>>> 314: } else {
>>
>> This can be `else if`
>
> The benefit of the current structure is that there's a clear separation
> between the static case and the non-static case. This also matches the layout
> of FieldPrinter::do_field.
>
> Spit-balling some other suggestions:
>
> if (obj == nullptr) {
> // Print static fields
> assert(vpc == nullptr, "flattening not supported for static fields");
> } else {
> // Print non-static fields
> if (vpc != nullptr) {
> assert(obj->klass() != vpc->klass(), "a value object cannot be
> flattened into itself");
> }
> }
>
> or (I thought I wrote something like this ...)
>
> if (obj == nullptr) {
> // Print static fields
> assert(vpc == nullptr, "flattening not supported for static fields");
> } else {
> // Print non-static fields
> assert(vpc == nullptr || obj->klass() != vpc->klass(), "a value object
> cannot be flattened into itself");
> }
>
> or (Probably taking this too far)
>
> assert(obj != nullptr || vpc == nullptr, "flattening not supported for
> static fields");
> assert(obj == nullptr || vpc == nullptr || obj->klass() != vpc->klass(),
> "a value object cannot be flattened into itself");
> }
I am not a fan of putting `||` in asserts. When the assert fails, you have to
mentally negate every condition in a list of `||` to see why you got there.
It's too much energy to waste when debugging.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3978106333
