On Thu, 31 Jul 2025 09:46:48 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org>
wrote:
>> src/java.base/share/classes/jdk/internal/ffi/util/FFMUtils.java line 51:
>>
>>> 49: public static final AddressLayout C_POINTER = ValueLayout.ADDRESS
>>> 50:
>>> .withTargetLayout(MemoryLayout.sequenceLayout(Long.MAX_VALUE, JAVA_BYTE));
>>> 51: public static final ValueLayout.OfLong C_LONG =
>>> (ValueLayout.OfLong) Linker.nativeLinker().canonicalLayouts().get("long");
>>
>> This seems problematic. On Windows, the C type `long` has a `JAVA_INT`
>> layout, because it only uses 32 bits. The reason jextract re-generates all
>> these constants is that they can, in general, vary by platform, so my
>> general feeling is that we can't just have a shared set of layout constants
>> at these level -- the constants will need to be added in the
>> platform-specific directories. If you want to make this layer more portable,
>> then the `C_LONG` constant should be dropped, and clients should use either
>> `C_INT` or `C_LONG_LONG` (which is what most portable C APIs end up doing
>> anyway).
>>
>> Another possible way to have a more robust shared layer is to use
>> definitions in stdint.h -- e.g. not `C_INT`, but `C_INT32_T` (but then you
>> might have issues, as the jextract-generated files you are importing depend
>> on names like `C_INT`).
>
> IMHO the more honest approach is to move these constants closer to the _h
> generated files that use them, as that's the way jextract intends them to be
> used.
(Also, the latest version of jextract uses `Linker.canonicalLayouts` to derive
these constants, which might be more robust longer term)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25546#discussion_r2245216232
