On Mon, 1 Sep 2025 09:00:32 GMT, Francesco Andreuzzi <[email protected]> wrote:
> In this PR I introduce some constants in `StackMapGenerator`. No change in
> logic is expected.
>
> Passes tests in `jdk/classfile`.
Thanks for pinging me. This must have got forgotten in the stream of emails.
src/java.base/share/classes/jdk/internal/classfile/impl/StackMapDecoder.java
line 145:
> 143: }
> 144: } else if (fr.stack().size() == 1 &&
> fr.locals().equals(prevLocals)) {
> 145: if (offsetDelta <= SAME_FRAME_END) { //same locals 1 stack
> item frame
This is better represented as
Suggestion:
if (offsetDelta <= SAME_LOCALS_1_STACK_ITEM_FRAME_END -
SAME_LOCALS_1_STACK_ITEM_FRAME_START) { //same locals 1 stack item frame
src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java
line 188:
> 186:
> 187: public static final int
> 188: SAME_FRAME_END = 63,
Can you add a comment on these constants that they are inclusive on both ends?
A lot of ranges in Java are open on their higher end, like range notations in
Arrays.copyOf or String.substring.
src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java
line 1256:
> 1254: }
> 1255: } else if (stackSize == 1 && localsSize ==
> prevFrame.localsSize && equals(locals, prevFrame.locals, localsSize)) {
> 1256: if (offsetDelta <= SAME_FRAME_END) { //same locals 1
> stack item frame
Suggestion:
if (offsetDelta <= SAME_LOCALS_1_STACK_ITEM_FRAME_END -
SAME_LOCALS_1_STACK_ITEM_FRAME_START) { //same locals 1 stack item frame
Same comment.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/27029#issuecomment-3262178996
PR Review Comment: https://git.openjdk.org/jdk/pull/27029#discussion_r2327393978
PR Review Comment: https://git.openjdk.org/jdk/pull/27029#discussion_r2327385656
PR Review Comment: https://git.openjdk.org/jdk/pull/27029#discussion_r2327398615
