RFR: 8357220: Introduce a BSMAttributeEntry struct [v2]

[Johan Sjölen]

On Wed, 21 May 2025 17:28:12 GMT, Lois Foltan <lfoltan at 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