On Tue, 10 Mar 2026 20:23:37 GMT, Tom Rodriguez <[email protected]> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add some comments and fix the test.
>
> src/hotspot/share/oops/generateOopMap.cpp line 1174:
>
>> 1172: }
>> 1173:
>> 1174: void GenerateOopMap::do_exception_edge(BytecodeStream* itr) {
>
> the declaration ordering here is making the diff really hard to read. I
> assume you didn't change do_exception_edge but can't easily tell. Maybe move
> exception_can_trap after do_exception_edge?
I don't like that this diff looks like this but we're not consistent in hotspot
- some people prefer small callee functions below the caller, but I prefer them
just above the caller.
I could try to add a comment that might make the diff less bad.
> test/hotspot/jtreg/vmTestbase/nsk/jdb/kill/kill003/kill003.java line 87:
>
>> 85: }
>> 86:
>> 87: reply = jdb.receiveReplyForWithMessageWait(JdbCommand.kill +
>> threads[0] + " " + DEBUGGEE_EXCEPTION,
>
> a comment explaining these steps wouldn't hurt. Something like "execution is
> now at a bytecode which isn't expecting to handle an exception so throw an
> async exception at it to make sure it's gets handled without crashing."
I added some comments, thank you for the suggested wording.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30171#discussion_r2917950879
PR Review Comment: https://git.openjdk.org/jdk/pull/30171#discussion_r2917940459
