On Thu, 9 May 2024 15:00:35 GMT, Maurizio Cimadamore <mcimadam...@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`).

src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 616:

> 614:      *
> 615:      * {@snippet lang = "java":
> 616:      * O = this.byteOffsetHandle(P).invokeExact(B, I1, I2, ... In);

Good catch

test/jdk/java/foreign/TestLayoutPaths.java line 36:

> 34: 
> 35: import java.lang.invoke.MethodHandle;
> 36: import java.lang.invoke.VarHandle;

Nit: Copyright year

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19158#discussion_r1595624831
PR Review Comment: https://git.openjdk.org/jdk/pull/19158#discussion_r1595624353

Reply via email to