On Tue, 5 Aug 2025 10:16:32 GMT, Johan Sjölen <jsjo...@openjdk.org> wrote:
>> src/hotspot/share/interpreter/bytecodeUtils.cpp line 1483:
>>
>>> 1481: // Is an explicit slot given?
>>> 1482: bool explicit_search = slot >= 0;
>>> 1483: if (explicit_search) {
>>
>> Suggestion:
>>
>> if (slot >= 0) {
>>
>> No need for the temporary local.
>
> If we don't adopt the enum I suggested, then I do prefer the naming of the
> condition through a variable. It gives you a hint of what the check is
> looking for, and not only what the check does.
That is what comments are for.
>> src/java.base/share/classes/java/lang/NullPointerException.java line 78:
>>
>>> 76: NullPointerException(int stackOffset, int searchSlot) {
>>> 77: extendedMessageState = setupCustomBackTrace(stackOffset,
>>> searchSlot);
>>> 78: this();
>>
>> I thought `this()` had to come first in a constructor? Is this new in 25 or
>> 26?
>
> https://openjdk.org/jeps/492
And now final - https://openjdk.org/jeps/513
Thanks
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2256053422
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2256047794
