On Wed, 21 Jun 2023 23:33:37 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> Improve the specification of `Linker.Option.firstVariadicArg`, by specifying
>> more clearly which index values are valid.
>
> Jorn Vernee has updated the pull request incrementally with one additional
> commit since the last revision:
>
> polish doc, review comments
src/java.base/share/classes/java/lang/foreign/Linker.java line 654:
> 652: * <ul>
> 653: * <li>{@code 0}, all arguments passed to the function are
> passed as variadic arguments</li>
> 654: * <li>{@code N}, none of the arguments passed to the function
> are passed as variadic argument</li>
Suggestion:
* <li>{@code N}, none of the arguments passed to the function are
passed as variadic argumenst</li>
src/java.base/share/classes/java/lang/foreign/Linker.java line 658:
> 656: * </ul>
> 657: * It is important to always use this linker option when linking
> a <a href=Linker.html#variadic-funcs>variadic
> 658: * function</a>, even when none of the arguments are passed as
> variadic arguments (the second case in the list
Suggestion:
* function</a>, even if no variadic argument is passed (the second
case in the list
src/java.base/share/classes/java/lang/foreign/Linker.java line 659:
> 657: * It is important to always use this linker option when linking
> a <a href=Linker.html#variadic-funcs>variadic
> 658: * function</a>, even when none of the arguments are passed as
> variadic arguments (the second case in the list
> 659: * above), as this affects the linking process on certain
> platforms.
Suggestion:
* above), as this might still affect the calling convention on certain
platforms.
src/java.base/share/classes/java/lang/foreign/Linker.java line 664:
> 662: * against which the index is validated is available.
> 663: *
> 664: * @param index the index indicating the start of the variadic
> arguments passed to the function described by
My general feeling is that the previous text was better here. We describe in
plenty details what this "index" is in the method javadoc. I think I'm more for
reverting the changes here.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14565#discussion_r1240079184
PR Review Comment: https://git.openjdk.org/jdk/pull/14565#discussion_r1240081468
PR Review Comment: https://git.openjdk.org/jdk/pull/14565#discussion_r1240083169
PR Review Comment: https://git.openjdk.org/jdk/pull/14565#discussion_r1240087788
