On Thu, 10 Sep 2026 10:29:38 GMT, Ioi Lam <[email protected]> wrote: >> 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.
Fair enough. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3978378577
