On Wed, 31 May 2023 23:35:08 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 368:
>
>> 366: * <h3 id="variadic-funcs">Variadic functions</h3>
>> 367: *
>> 368: * Variadic functions (e.g. a C function declared with a trailing
>> ellipses {@code ...} at the end of the formal parameter
>
> Optional suggestion for improvement (maybe now, or maybe later). When reading
> this great para, I understand that there are two things that fall under the
> "variadic function" umbrella. Some are declared with `...` and some with
> `()`. This is a very good definition and I wonder if we should expand a bit
> more on it - e.g. in a way, we never explain what a variadic function is - we
> merely define it by saying how it is declared in C. I wonder if we might very
> very briefly explain that in C, some functions (variadic functions) can take
> a variable number of parameters. Then we go on to say that these functions
> can be declared in two ways (e.g. ellipsis and prototype-less). Perhaps if we
> used a bullet-list to define the two ways in which variadic function can be
> declared, the definition could stand out even more?
Also, should we say somewhere that, for prototype-less functions,
`firstVariadicArg` should always have an index of 0 (e.g. any other value is
illegal, trivially). It's not super important, just one of those little things
that can reinforce understanding.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14225#discussion_r1212416524