On Mon, 18 Sep 2023 14:17:30 GMT, Jorn Vernee <jver...@openjdk.org> wrote:

>> This patch contains the implementation of the foreign linker & memory API 
>> JEP for Java 22. The initial patch is composed of commits brought over 
>> directly from the [panama-foreign 
>> repo](https://github.com/openjdk/panama-foreign). The main changes found in 
>> this patch come from the following PRs:
>> 
>> 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous 
>> iterations supported converting Java strings to and from native strings in 
>> the UTF-8 encoding, we've extended the supported encoding to all the 
>> encodings found in the `java.nio.charset.StandardCharsets` class.
>> 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the 
>> `MemoryLayout::sequenceLayout` factory method which inferred the size of the 
>> sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A 
>> client is now required to explicitly specify the sequence size.
>> 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: 
>> `Linker::canonicalLayouts`, which exposes a map containing the 
>> platform-specific mappings of common C type names to memory layouts.
>> 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access 
>> varhandles, as well as byte offset and slice handles derived from memory 
>> layouts, now feature an additional 'base offset' coordinate that is added to 
>> the offset computed by the handle. This allows composing these handles with 
>> other offset computation strategies that may not be based on the same memory 
>> layout. This addresses use-cases where clients are working with 'dynamic' 
>> layouts, whose size might not be known statically, such as variable length 
>> arrays, or variable size matrices.
>> 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now 
>> redundant API. Clients can simply use the difference between the base 
>> address of two memory segments.
>> 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of 
>> `SegmentAllocator::allocateArray`, by renaming methods that both allocate + 
>> initialize memory segments to `allocateFrom`. (see the original PR for the 
>> problematic case)
>> 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the 
>> documentation for variadic functions.
>> 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to 
>> make sure the `jdk_foreign` tests can pass on 32-bit machines, taking 
>> linux-x86 as a test bed.
>> 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API 
>> required. The `Linker::nativeLinker` method is not longer allowed to throw 
>> an `UnsupportedO...
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Avoid eager use of LibFallback in FallbackLinker static block

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

> 150:  * <p>
> 151:  * The following table shows some examples of how C types are modelled 
> in Linux/x64 according to the
> 152:  * "System V Application Binary Interface - AMD64 Architecture Processor 
> Supplement" (all the examples provided

I have seen some discussion on this and I agree an authoritative link does not 
exist (I have searched for it in the past). I guess I'm not super wild about 
also including "AMD64 Architecture Processor Supplement". E.g. IMHO "System V 
Application Binary Interface" is more than enough?

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

> 407:  *
> 408:  * Variadic functions are C functions which can accept a variable number 
> and type of arguments. They are declared with a
> 409:  * trailing ellipsis ({@code ...}) at the end of the formal parameter 
> list, such as: {@code void foo(int x, ...);}.

Looking at the javadoc - it seems to me that the `;` after the declaration of 
`foo` leads to a bit of jarring as it is immediately followed by a period 
(`.`). Consider dropping that - or maybe put the declaration in a snippet.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15103#discussion_r1334395919
PR Review Comment: https://git.openjdk.org/jdk/pull/15103#discussion_r1334399541

Reply via email to