On Wed, 21 May 2025 17:28:12 GMT, Lois Foltan <lfol...@openjdk.org> wrote:
>> Johan Sjölen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Lois's comments
>
> src/hotspot/share/oops/constantPool.hpp line 80:
>
>> 78: };
>> 79:
>> 80: class BSMAttributeEntry {
>
> Please consider moving the description comment from lines #569-#572 ahead of
> this structure
This?
// The first part of the operands array consists of an index into the second
part.
// Extract a 32-bit index value from the first part.
Wouldn't it be better to leave it where it is, but add a comment ahead of the
structure?
> src/hotspot/share/oops/constantPool.hpp line 92:
>
>> 90: return reinterpret_cast<u2*>(this + 1);
>> 91: }
>> 92:
>
> Remove blank lines at #92, #95, #103
We typically keep a space before we change access modifier, let's keep that
pattern in here. We can tighten up the rest of the code according to your
suggestions, however.
> src/hotspot/share/oops/constantPool.hpp line 105:
>
>> 103:
>> 104: int argument_index(int n) const {
>> 105: assert(checked_cast<u2>(n) < _argument_count, "oob");
>
> Should the assert contain a check that _argument_count is >= 0 as well?
I don't think so, it's a `u2` so that's just part of its type.
> src/hotspot/share/oops/constantPool.hpp line 638:
>
>> 636: u2 bootstrap_argument_count_at(int cp_index) {
>> 637: assert(tag_at(cp_index).has_bootstrap(), "Corrupted constant pool");
>> 638: int bsmai = bootstrap_methods_attribute_index(cp_index);
>
> The "matched ending" assert was the only code that used
> bootstrap_operand_limit(), so that method could be removed as well. This
> comment applies to bootstrap_operand_base() as well.
Nice!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2103107280
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2103101655
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2103103526
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2103104346
