[foreign-abi] RFR: 8248420: Add a variant of VaList::make which takes a NativeScope [v2]

[Jorn Vernee]

On Fri, 10 Jul 2020 16:39:51 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> The API changes look good. I think some follow on work is required in the JavaDoc.
>> 
>> It's tempting to consider a `CNativeScope` extending `NativeScope`, perhaps making it slightly easier to allocate c
>> strings, va_lists etc. Something to consider later maybe.
>> It's been a while since i have used `va_list` and needed to refamiliarize myself with it :-)
>> 
>> I think the documentation could make clearer the side-effecting nature of obtaining arguments, and that the copy is a
>> copying the va_list at its current position.
>> `vargAsInt` reads the *next* argument as an int.
>> 
>> Referring to the copy as a view is likely misleading to Java developers. va_list is more like a cursor or iterator, and
>> its current state can be copied. A copy in this respect is useful for subsequent traversal at the current position, and
>> may be called at any point while traversing a va_list to traverse from the same position without affecting the position
>> of the source va_list and vice versa.
>
> Thanks for the comments, I'm working on updating the javadoc, but found another bug after adding extra tests that I'm
> also resolving.

I've tried to update the javadoc to clarify the statefulness of the va list, as well as the bahaviours of the the make,
copy, and close methods with regards to memory allocation. Please find the javadoc changes here:
https://github.com/openjdk/panama-foreign/pull/237/commits/e351a137a24c1d789b84328799bb3456e85c353c

I've also added tests to verify that operating on a va list copy, whose original has been closed will fail (fixing the
bug in the implementation that this revealed).

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

PR: https://git.openjdk.java.net/panama-foreign/pull/237