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

[Jorn Vernee]

On Wed, 15 Jul 2020 00:38:47 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update CSupport.java
>>   
>>   Typo
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CSupport.java line 161:
> 
>> 160:          * of a va list does not need any native memory, so nothing has to be released. After calling {@code
>> close()} on 161:          * such an instance {@link #isAlive()} will still return {@code true}.
>> 162:          *
> 
> Is it possible hide the implementation detail so close behaviour is consistent across all implementations, even if on
> some implementation no allocation is required? Especially since close also may close any native memory that is attached.

I think it's possible by creating a dummy segment when doing the copy on Windows. The dummy segment can then be used to
check if the VaList is alive or not, and we can also register it with the NativeScope so it gets closed by that. Feels
kinda hacky though.

A proper solution is probably to create a Resource abstraction that has a close() method, as well as a way to transfer
the resource to another owner (i.e. the NativeScope). Then we can have a register(Resource) method on NativeScope that
transfers the resource to the scope. One of the problems currently is that NativeScope doesn't call the close() method
of VaList, but the close() method of the MemorySegment that we allocate through it, so we have to piggy back on the
current NativeScope capabilities, or create an internal-only way of registering a VaList with it (the latter feels
kinda hacky again).

> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CSupport.java line 72:
> 
>> 71:         /**
>> 72:          * Reads the next value into an {@code int} and advances this va list's position.
>> 73:          *
> 
> For primitives values i recommend saying "Reads the next value as an {@code int), and ...".
> 
> What happens if the layout is not compatible with the carrier type `int.class`?

Ok, I'll change 'into an' -> 'as an'.

If the layout and carrier type are incompatible an IllegalArgumentException is thrown. I'll add a @throws tag to those
methods as well.

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

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