[foreign-memaccess+abi] RFR: 8312059: Clarify the documention for variadic functions

Maurizio Cimadamore mcimadamore at openjdk.org
Fri Jul 14 17:22:23 UTC 2023


On Thu, 13 Jul 2023 21:53:03 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> This patch attempts to refine the javadoc for variadic functions further. Particularly, it tries to make it clearer that the layouts the native linker rejects are derived from the C specification. Instead of trying to say which layouts are reject, we say which C types are rejected, and then give a hint about how to map those C types into layouts, using `canonicalLayouts()`, which would tell a client which layouts are rejected.
> 
> Additionally, this patch removes the mentions of prototype-less functions, which are not considered variadic functions in C. It is also not clear whether arguments passed to such a function should be treated as variadic or not in the ABI (see linked issue). Finally the latest C standard, C23, prototype-less functions no longer exist, and a declaration of the form `void foo()` instead means the same as `void foo(void)`. For these reason, passing arguments to prototype-less functions will also not be supported (officially) by the linker. I wasn't sure about whether to add a specific note about that. I figured that just not saying they are supported, combined with the state of the latest C standard, would make this obvious enough (?).

Good work - some minor comments

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

> 406:  * <h3 id="variadic-funcs">Variadic functions</h3>
> 407:  *
> 408:  * Variadic functions are C functions which can accept a variable number and type of arguments. They are declared With a

Suggestion:

 * Variadic functions are C functions which can accept a variable number and type of arguments. They are declared with a

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

> 422:  * whereby the signed-ness of the source type corresponds to the signed-ness of the promoted type. The complete process
> 423:  * of default argument promotion is described in the C specification. In effect these promotions place limits on the
> 424:  * types that can be used to replace the {@code ...}, as the variadic parameters of the specialized form will always

specialized form -> specialized function? (this was there even before, but I'm not sure "form" is too clear given previous terminology)

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

> 431:  * function descriptor, are called <em>variadic argument layouts</em>.
> 432:  * <p>
> 433:  * The native linker does not automatically perform default argument promotions, since it would be ambiguous whether sign

I think we should drop the "since it would be ambiguous...". Here we should stress how things work, and less why they are the way they are (e.g. the signedness ambiguity might be lifted if we had different primitive types).

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

> 434:  * extension should take place. However, since passing an argument of a non-promoted type as a variadic argument is not
> 435:  * supported in C, the native linker will reject an attempt to link a specialized function descriptor with any variadic
> 436:  * argument layouts corresponding to a non-promoted C type. Since the size of the C {@code int} type is platform-specific,

This is slightly imprecise - although I know what you mean - it does seem to imply that if I pass a struct layout it will be rejected.

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

PR Review: https://git.openjdk.org/panama-foreign/pull/846#pullrequestreview-1530667981
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/846#discussion_r1263959765
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/846#discussion_r1263961265
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/846#discussion_r1263964317
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/846#discussion_r1263965365


More information about the panama-dev mailing list