On Thu, 19 Oct 2023 16:52:53 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> 
wrote:

> The 21 iteration of the FFM API used to contain a method, namely 
> `ValueLayout::arrayElementVarHandle`. This var handle factory was useful when 
> accessing elements in a flat array of variable size.
> 
> In 22, this method has been removed because:
> * It was not general enough (it only worked with value layouts)
> * A more useful/general version could be obtained with the following code:
> 
> 
> MethodHandles.collectCoordinates(varHandle(elements), 1, scaleHandle());
> 
> 
> Given that accessing variable-length array is rather common, and given that 
> combining var handles is perceived as hard, we noticed that developers often 
> tried to workaround by creating a sequence layout of maximal size, and then 
> deriving a var handle from it. This feels suboptimal (as a new layout is 
> created just so that we can select from it, and using this layout for 
> anything else will likely lead to exceptions, because of its size).
> 
> For these reasons, we would like to expose this functionality as a new method 
> in the `MemoryLayout` interface, namely 
> `MemoryLayout::arrayElementVarHandle`. This method generalizes the old method 
> in the 21 API, and is implemented in the obvious way, using a combinator (see 
> above).
> 
> This PR also adds a lot of narrative text to describe how to deal with 
> variable-length arrays, which should clarify the role of the new method.

Marked as reviewed by jvernee (Reviewer).

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

> 332:  * }
> 333:  *
> 334:  * Here, the coordinate {@code x} of subsequent point in the array are 
> accessed using the {@code POINT_ARR_X} var

Suggestion:

 * Here, the coordinate {@code x} of subsequent points in the array is accessed 
using the {@code POINT_ARR_X} var

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 137:

> 135:  *
> 136:  * {@snippet lang=java:
> 137:  * MethodHandle scale = ValueLayout.JAVA_INT.scaleHandle();

`scale` seems unused now

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

PR Review: https://git.openjdk.org/jdk/pull/16272#pullrequestreview-1688419431
PR Review Comment: https://git.openjdk.org/jdk/pull/16272#discussion_r1365965777
PR Review Comment: https://git.openjdk.org/jdk/pull/16272#discussion_r1365981020

Reply via email to