On Wed, 31 May 2023 23:41:25 GMT, Maurizio Cimadamore <[email protected]>
wrote:
>> Jorn Vernee has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> review comments
>
> src/java.base/share/classes/java/lang/foreign/Linker.java line 379:
>
>> 377: * type {@code float} is converted to {@code double}, and each integral
>> type smaller than {@code int} is converted to
>> 378: * {@code int}. As such, the native linker will reject attempts to link
>> function descriptors with certain variadic argument
>> 379: * layouts. Namely, {@linkplain ValueLayout value layouts} that have a
>> carrier type of {@code boolean}, {@code byte},
>
> Is there any reason as to why you decided to say which layouts are **not**
> allowed as variadic layouts? I'm wondering whether, if we add more carriers
> in the future, this list will probably need to be updated? Or do the
> restriction only involve these "small" types, and stuff like `long double` is
> always allowed? (in which case the set of unsupported layouts would remain
> stable over time)
No real reason for the current polarity. The specification explicitly calls out
the float -> double conversion, and then for the integral types it refers to
['integer
promotions'](https://en.cppreference.com/w/c/language/conversion#Integer_promotions),
which is a somewhat complex rule set for assigning ranks to integer types.
So, I think if we add more carriers in the future, we would have to worry about
integral types that have a rank less than (unsigned) int when considering this
list. This is currently only the types I've listed (though, `char` is a bit of
a special case), and this seems relatively stable to me. e.g. `long double`
would not need to be added here.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14225#discussion_r1212432229
