[foreign-memaccess+abi] RFR: 8298096: Refine javadoc for Linker

[Jorn Vernee]

On Thu, 16 Mar 2023 14:58:11 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> The javadoc for the `Linker` interface is very abstract: it defines the general characteristics of downcall method handles and upcall stubs, and it does so in a very general way, so as not to bias the discussion towards a specific kind of linker implementation.
> 
> The result is that, looking at the `Linker` javadoc, one has very little clues on how this interface is meant to be used e.g. to write native interop code. While the toplevel package javadoc provides some examples, these examples are relatively simple and do not cover all the features provided by the FFM API.
> 
> To rectify this, I did another pass on the `Linker` javadoc. I retained the very good general intro (the first few paras). But then, instead of talking about "downcalls" and "upcalls" - we immediately start talking about "Calling native functions" and we ground the discussion on the native linker. This gives us the opportunity to discuss native calls, function pointers, variadic calls, and even how to deal with functions returning pointers.
> 
> The text of the sections on upcalls/downcall (which is normative text) has been moved in the javadoc of the relevant methods.
> 
> Finally, since with the new examples there was some overlapping between the toplevel package javadoc and the `Linker` javadoc, I also decided to take another pass at the package javadoc, and simplify it further: now there are only 3 sections: foreign memory, foreign function, restricted methods. For each of the first two sections there's a quick summary (which describes the main abstractions and their roles), followed by an idiomatic example. Restricted methods also belong here, as they are a cross-cutting concern of the FFM API.
> 
> Overall, I believe this iteration is much better than what we had, and more useful. While covering all the details of native interop would be outside the scope of the FFM javadoc, I think the new `Linker` javadoc strikes a good balance.

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

> 111:  * maps to a {@linkplain StructLayout struct layout}, whereas a C {@code union} type maps to a {@link UnionLayout union
> 112:  * layout}. Depending on the ABI implemented by the native linker, additional {@linkplain MemoryLayout#paddingLayout(long) padding}
> 113:  * member layouts might be required to conform to the size and alignment constraint of a composite type definition in C.

I think the comment on padding here is maybe a bit too vague. I think it would be better to say explicitly that padding is not added to group layouts automatically, and has to be manually specified.

(On that note, maybe we want to add a factory in MemoryLayout that basically exposes `Utils.computePaddedStruct`)

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

> 173:      * Restricted methods are unsafe, and, if used incorrectly, their use might crash
> 174:      * the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
> 175:      * restricted methods, and use safe and supported functionalities, where possible.

Shouldn't the 'restricted' paragraph stay?

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

> 206:  *
> 207:  * The {@code qsort} function can be used to sort the contents of an array, using a custom comparator function which is
> 208:  * passed as a function pointer (the {@code compar} parameter). To be able to call {@code qsort} function from Java,

Suggestion:

 * passed as a function pointer (the {@code compar} parameter). To be able to call the {@code qsort} function from Java,

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

> 321:  * {@snippet lang = java:
> 322:  * MemorySegment allocateMemory(long byteSize, Arena arena) {
> 323:  *     MemorySegment segment = (MemorySegment)malloc.invokeExact(byteSize);   // size = byteSize, scope = always alive

Suggestion:

 *     MemorySegment segment = (MemorySegment)malloc.invokeExact(byteSize);   // size = 0, scope = always alive

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

PR: https://git.openjdk.org/panama-foreign/pull/820