Classfile API ConstantPool::entryCount and ConstantPool::entryByIndex confusion

[Brian Goetz]

These seem reasonable. My preference is for throwing over optional.  Size is probably the least problematic name.

Sent from my iPad

On Aug 11, 2023, at 2:10 AM, Adam Sotona <adam.sotona at oracle.com> wrote:


Hi,
I’ve noticed confusion in understanding (and inconsistency in implementations) of two ConstantPool methods:
    /**
     * {@return the entry at the specified index}
     *
     * @param index the index within the pool of the desired entry
     */
    PoolEntry entryByIndex(int index);

    /**
     * {@return the number of entries in the constant pool}
     */
    int entryCount();


Intuitive understanding of the above methods is that user can iterate over entries incrementing index by one up-to the entryCount and get entry for each index from the range.

However the reality is that the methods reflects JVMS 4.1:

constant_pool_count

The value of the constant_pool_count item is equal to the number of entries in the constant_pool table plus one. A constant_pool index is considered valid if it is greater than zero and less than constant_pool_count, with the exception for constants of type long and double noted in §4.4.5<https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.4.5>.

Following user code cause more or less confusion:



for (int i = 0; i < cp.entryCount(); i++) cp.entryByIndex(i);

  *   Fails immediately with ConstantPoolException at index 0


for (int i = 1; i < cp.entryCount(); i++) cp.entryByIndex(i);

  *   May fail for constant pools containing long or double entries (double-slot entries), however it may not fail if the tag at the invalid offset imitates a valid entry (this is a bug) or it may return null when SplitConstantPool implementation is involved (inconsistency in implementations).

So the only valid (however not very intuitive) iteration over all entries should look like this:



for (int i = 1; i < cp.entryCount(); i += cp.entryByIndex(i).width())


I propose following changes to ConstantPool:

  *   Fix all implementations of PoolEntry ConstantPool::entryByIndex(int) to always throw ConstantPoolException when the index is invalid
or change the method signature to Optional<PoolEntry> ConstantPool::entryByIndex(int) and explain it in the Javadoc
  *   Rename ConstantPool::entryCount to slotsCount or size or width and explain it in the Javadoc
  *   Make ConstantPool extends Iterable<PoolEntry> so user does not need to understand CP internals to iterate over its entries

Thanks,
Adam


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20230811/97682092/attachment-0001.htm>