On Wed, 31 May 2023 22:44:52 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> In C, arguments smaller than `int` are promoted to (`unsigned`) `int`, and >> `float` is promoted to `double`, when being passed as variadic argument (see >> e.g. >> https://en.cppreference.com/w/c/language/conversion#Default_argument_promotions). >> This patch restricts the layouts that can be used as variadic layouts to >> what is allowed by the C specification. >> >> The fallback linker is also updated to use to correct function to link >> variadic calls (not doing this turned out not to be a problem so far, but it >> is problematic for instance on Mac/AArch64 when using the fallback linker). >> Adding the restriction on layouts for all linkers is also partly motivated >> by the fallback linker rejecting such unsupported variadic layouts already. >> >> I've added a small paragraph to the Linker javadoc as well that explains the >> restriction. Comments on that are welcome, but please explain. >> >> The tests are updated to no longer try to link variadic functions with the >> illegal layouts, and I've added some more negative tests to TestIllegalLink. >> >> Testing: >> - local testing on Windows/x64 >> - tier1-3 + jdk-tier5 (ongoing) >> - manual test run on mac/aarch64 with the fallback linker to verify the >> correctness of the fallback linker changes. > > 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) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14225#discussion_r1212419388
