On Fri, 14 Aug 2026 20:34:07 GMT, Matias Saavedra Silva <[email protected]> 
wrote:

>> Coleen Phillimore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Change redefine deleted name
>
> src/hotspot/share/classfile/javaStackTraceClasses.cpp line 247:
> 
>> 245: 
>> 246:  private:
>> 247:   // Move this up.
> 
> Leftover comment here

That comment was a reminder that after the first round of reviews, I want that 
method to be in the private part of the BacktraceBuilder, instead of the middle 
of the public part.
I didn't move it because it would make it harder to see the diffs.  Now I've 
moved it.

> src/hotspot/share/classfile/javaStackTraceClasses.cpp line 291:
> 
>> 289:     }
>> 290: 
>> 291:     _methods_and_bcis->long_at_put(_index,
> 
> Indentation seems off here

This looks right in the source code.  It's indented by 4.

> src/hotspot/share/classfile/javaStackTraceClasses.cpp line 769:
> 
>> 767: }
>> 768: 
>> 769: 
> 
> Extra line deleted here

This blank line should be deleted.  It's somewhat related since I touched 
somewhat nearby code.

> src/hotspot/share/classfile/javaStackTraceClasses.cpp line 1222:
> 
>> 1220: }
>> 1221: 
>> 1222: 
> 
> Extra line deleted

This one could go back since the blank line separates code for different 
classes.

> test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineRunningMethodsWithBacktrace.java
>  line 137:
> 
>> 135:         // The first version of this method is running so we have the 
>> line number,
>> 136:         // the second is cleaned up, so we don't, the third version is 
>> current so we do.
>> 137:         return lineNumber == 82 || lineNumber == -1 || lineNumber == 13;
> 
> Can these be fields instead of constants? I'm concerned that some change 
> elsewhere could cascade and change the line numbers down the line.

Okay added some static consts that are a bit more descriptive.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/32202#discussion_r3804077906
PR Review Comment: https://git.openjdk.org/jdk/pull/32202#discussion_r3804062613
PR Review Comment: https://git.openjdk.org/jdk/pull/32202#discussion_r3804051624
PR Review Comment: https://git.openjdk.org/jdk/pull/32202#discussion_r3804047295
PR Review Comment: https://git.openjdk.org/jdk/pull/32202#discussion_r3804107438

Reply via email to