On Fri, 8 Sep 2023 10:27:28 GMT, Adam Sotona <[email protected]> wrote:
>> Classfile API `ConstantPool::entryCount` and `ConstantPool::entryByIndex`
>> methods are confusing and unsafe to use without knowledge of constant pool
>> specification.
>> This patch renames `ConstantPool::entryCount` to `ConstantPool::size` and
>> adds checks to `ConstantPool::entryByIndex` for invalid or unusable indexes.
>> `ConstantPool` newly extends `Iterable<PoolEntry>` for simplified user
>> iteration over all pool entries.
>> Range checks for invalid index have been added also to
>> `ConstantPoo::bootstrapMethodEntry` methods and several `@jvms` javadoc tags
>> have been added to pool entries.
>>
>> Please review.
>>
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains four additional
> commits since the last revision:
>
> - fixed javac tests
> - Merge branch 'master' into JDK-8315678-cp-iterable
> - fixed tests
> - 8315678: Classfile API ConstantPool::entryCount and
> ConstantPool::entryByIndex is confusing
Marked as reviewed by briangoetz (Reviewer).
src/java.base/share/classes/jdk/internal/classfile/constantpool/ConstantPool.java
line 51:
> 49:
> 50: /**
> 51: * {@return the size of the constant pool}
There is still some confusion over the meaning of this method, as "size" (as
well as "entry count") could refer to either (a) the number of slots in the
constant pool or (b) the number of actual entries in the constant pool, since
Constant_{Long,Double} can use two slots. I agree with the name "size" but we
should further clarify that this is the number of slots, but that (a) not all
slots necessarily correspond to a valid entry (and therefore entryByIndex may
fail) and (b) that iterating the pool may yield fewer entries than the size.
-------------
PR Review: https://git.openjdk.org/jdk/pull/15567#pullrequestreview-1627302869
PR Review Comment: https://git.openjdk.org/jdk/pull/15567#discussion_r1326210410
