RFR: 8332003: Clarify javadoc for MemoryLayout::offsetHandle [v2]
Maurizio Cimadamore
mcimadamore at openjdk.org
Thu May 9 18:31:52 UTC 2024
On Thu, 9 May 2024 18:26:17 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
>> Consider this layout:
>>
>>
>> MemoryLayout SEQ = MemoryLayout.sequenceLayout(5,
>> MemoryLayout.sequenceLayout(10, JAVA_INT));
>>
>>
>> And the corresponding offset handle:
>>
>>
>> MethodHandle offsetHandle = SEQ.offsetHandle(PathElement.sequenceLayout(), PathElement.sequenceLayout());
>>
>>
>> The resulting method handle takes two additional `long` indices. The implementation checks that the dynamically provided indices conform to the bound available at construction: that is, the first index must be < 5, while the second must be < 10. If this is not true, an `IndexOutOfBoundException` is thrown.
>>
>> However, the javadoc for `MemoryLayout::byteOffsetHandle` doesn't claim anything in this direction. There are only some vague claims in the javadoc for `PathElement::sequenceElement()` and `PathElement::sequenceElement(long, long, long)` which make some claims on which indices are actually allowed, but the text seems more in the tone of a discussion, rather than actual normative text.
>>
>> I've tweaked the javadoc for `MemoryLayout::byteOffsetHandle` to actually state that the indices will be checked against the "size" of the corresponding open path element (this is a new concept that I also have defined in the javadoc).
>>
>> I also added a test for `byteOffsetHandle` as I don't think we had a test for that specifically (although this method is tested indirectly, via `MemoryLayout::varHandle`).
>
> Maurizio Cimadamore has updated the pull request incrementally with two additional commits since the last revision:
>
> - Update copyright
> - Add javadoc to other MemoryLayout methods returning VarHandle/MethodHandle to describe which exception can be thrown by returned handle
In an offline discussion @minborg noted that other methods in the `MemoryLayout` class do not specify in full the set exception thrown by the returned method handle/var handle:
* MemoryLayout::varHandle
* MemoryLayout::sliceHandle
* MemoryLayout::arrayElementVarHandle
The first two are missing details re. checks against the upper bound of the index parameter/coordinates. I believe this omission was because these methods already say that if the accessed offset is bigger than the segment size, then a IIOBE occurs. But there's a subtle problem with it. In the impl, even if you have a big enough memory segment to allow access at index 42, but the layout says that the max index is 41, access fails with IIOBE anyway. So I think the javadoc should be updated to contain the new text about the index checks.
The last method in the list actually doesn't have any text when it comes to the exceptions thrown by the returned handle. This is because we say that this method can be obtained by calling `collectCoordinates` on `MemoryLayout::varHandle`. Nevertheless, I think it's worth repeating what exceptions can come up (especially as doing so reveals more clearly that there's no extra checks for the very first long coordinate of the returned handle - so now the existing `apiNote` makes even more sense, I think).
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19158#issuecomment-2103200882
More information about the core-libs-dev
mailing list