RFR: 8295044: Implementation of Foreign Function and Memory API (Second Preview) [v9]

[Jorn Vernee]

On Tue, 8 Nov 2022 13:28:58 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This PR contains the API and implementation changes for JEP-434 [1]. A more detailed description of such changes, to avoid repetitions during the review process, is included as a separate comment.
>> 
>> [1] - https://openjdk.org/jeps/434
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Rework package-level javadoc for restricted methods

Did a full review. Only some minor comments.

Also, please add attribution with `/contributor add @<user>` for the people that contributed. (I think you have to add yourself as well, if you do that).

src/java.base/share/classes/java/lang/foreign/GroupLayout.java line 57:

> 55: 
> 56:     @Override
> 57:     GroupLayout withName(String name);

It looks like this method, and `withBitAlignment` below have no javadoc? Does this need `inheritDoc`?

src/java.base/share/classes/java/lang/foreign/Linker.java line 75:

> 73:  * <ul>
> 74:  * <li>if {@code L} is a {@link ValueLayout} with carrier {@code E} then {@code C = E}; or</li>
> 75:  * <li>if {@code L} is a {@link GroupLayout}, then {@code C} is set to {@code MemorySegment.class}</li>

Now that we have `FunctionDescriptor::toMethodType` I think this paragraph could be simplified by just referencing that.

src/java.base/share/classes/java/lang/foreign/Linker.java line 101:

> 99:  * <ul>
> 100:  * <li>if {@code L} is a {@link ValueLayout} with carrier {@code E} then {@code C = E}; or</li>
> 101:  * <li>if {@code L} is a {@link GroupLayout}, then {@code C} is set to {@code MemorySegment.class}</li>

Same here. This is covered by the doc of `FunctionDescriptor::toMethodType`.

src/java.base/share/classes/java/lang/foreign/Linker.java line 119:

> 117:  *     <li>The memory session of {@code A} is {@linkplain MemorySession#isAlive() alive}. Otherwise, the invocation throws
> 118:  *     {@link IllegalStateException};</li>
> 119:  *     <li>The invocation occurs in same thread as the one {@linkplain MemorySession#isOwnedBy(Thread) owning} the memory session of {@code R},

Suggestion:

 *     <li>The invocation occurs in same thread as the one {@linkplain MemorySession#isOwnedBy(Thread) owning} the memory session of {@code A},

src/java.base/share/classes/java/lang/foreign/Linker.java line 121:

> 119:  *     <li>The invocation occurs in same thread as the one {@linkplain MemorySession#isOwnedBy(Thread) owning} the memory session of {@code R},
> 120:  *     if said session is confined. Otherwise, the invocation throws {@link WrongThreadException}; and</li>
> 121:  *     <li>The memory session of {@code R} is <em>kept alive</em> (and cannot be closed) during the invocation.</li>

Suggestion:

 *     <li>The memory session of {@code A} is <em>kept alive</em> (and cannot be closed) during the invocation.</li>

src/java.base/share/classes/java/lang/foreign/StructLayout.java line 43:

> 41: 
> 42:     @Override
> 43:     StructLayout withName(String name);

Missing `inheritDoc`?

src/java.base/share/classes/java/lang/foreign/UnionLayout.java line 43:

> 41: 
> 42:     @Override
> 43:     UnionLayout withName(String name);

Missing `inheritDoc`?

src/java.base/share/classes/java/lang/foreign/VaList.java line 44:

> 42:  * Helper class to create and manipulate variable argument lists, similar in functionality to a C {@code va_list}.
> 43:  * <p>
> 44:  * A variable argument list segment can be created using the {@link #make(Consumer, MemorySession)} factory, as follows:

Suggestion:

 * A variable argument list can be created using the {@link #make(Consumer, MemorySession)} factory, as follows:

src/java.base/share/classes/java/lang/foreign/VaList.java line 50:

> 48:  *                                           .addVarg(C_DOUBLE, 3.8d));
> 49:  *}
> 50:  * Once created, clients can obtain the platform-dependent {@linkplain #segment() memory segment} associated a variable

Suggestion:

 * Once created, clients can obtain the platform-dependent {@linkplain #segment() memory segment} associated with a variable

src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 134:

> 132: 
> 133:     @Override
> 134:     ValueLayout withName(String name);

Missing `inheritDoc` here as well, and on other withers below.

src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 356:

> 354:      * Equivalent to the following code:
> 355:      * {@snippet lang=java :
> 356:      * ADDRESS.of(ByteOrder.nativeOrder())

This code doesn't look correct. It also looks like OfAddress layouts have their alignment set to the address size already, so the alignment adjustment here seems unnecessary as well.

src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 367:

> 365:      * Equivalent to the following code:
> 366:      * {@snippet lang=java :
> 367:      * JAVA_BYTE.of(ByteOrder.nativeOrder()).withBitAlignment(8);

Same here (and for the other snippets below), `OfByte` doesn't have an `of` method. This looks maybe like a regex-replace error.

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

PR: https://git.openjdk.org/jdk/pull/10872