On Mon, 5 Jun 2023 20:27:42 GMT, Vladimir Ivanov <[email protected]> wrote:
>>> Why is it limited to non-product builds? It's valuable irrespective of
>>> build flavor.
>>
>> This is because `print_on` in `AnyObj` is only defined in non-product
>> builds. I based implementation of `ObjectMergeValue::print_on` on
>> `ObjectValue::print_on`. In `ObjectValue::print_on` fields aren't printed in
>> product builds.
>>
>>> Any particular reason for that? You could add is_object_merge case in
>>> ObjectValue::print_oninstead and extendObjectValue::print_fields_onto
>>> coverObjectMergeValue case.
>>
>> I'll do that then.
>>
>>> Also, formatting is broken.
>>
>> Can you please share an example? If you mean the tabs on lines
>> 303/304/306/307 I added those because I thought would make the code easier
>> to read, but if you want I can definitely remove that.
>
>> If you mean the tabs on lines 303/304/306/307
>
> Yes, it confused me. As an alternative, you could put selector and
> merge_pointer-related statements on the same line, but I'm not sure how much
> it improves readability:
>
> st->print(", selector=""); _selector->print_on(st); st->print(""");
A couple of suggestions about the output:
* `merge`: it's clearer to call it `merge_obj`
* `obj` vs `merge` output: obj output is duplicated in ScopeDesc entries and
Objects sections; before it was a short version printed in Locals/Expressions
and all the details were included in Objects; I like to see field locations in
the short version, but including everything looks way too much IMO;
* it makes sense to include selector and merge_pointer info in short version,
but `is_root` can be omitted
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12897#discussion_r1218558295
