[foreign-jextract] RFR: Test the Java VaList implementation on every platform

Maurizio Cimadamore mcimadamore at openjdk.java.net
Fri Jul 17 09:15:23 UTC 2020


On Fri, 17 Jul 2020 00:17:14 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

>> Hi,
>> 
>> This patch re-writes the VaListTest to try and test all the implementations on the different platforms, instead of just
>> the implementation for the current platform.
>> This results in 4 different versions of each test being run; once for each platform, where the reading of the VaList is
>> done in Java as well, and then one native implementation (which corresponds to the status quo). The needed parameters
>> are injected into tests using data providers. For the struct tests this was a little tricky, since we need to inject
>> the platform specific C_INT/C_LONGLONG/C_FLOAT layouts into the struct layout. This resulted in the current code
>> complexity. (As an aside, I think local interface and methods work from Amber could really help this use-case).
>> Upcalls are still only tested for the current platform, since the creation of the va_list happens in native code, and
>> we only have access to 1 platform in that case :)
>> ---
>> 
>> I've also cleaned up the sumStack test, which wasn't actually using any part of our VaList implementation (it was
>> testing varargs), as well as reduced some copy-pasted code which was previously used for debugging, but never collapsed
>> back into for-loops afterwards.  Thanks,
>> Jorn
>
> There are some test methods and data providers with "win" in the name. Also, i don't fully understand the distinction
> between value and reference, i am guessing it's due to the size of the struct that changes the platform specific
> encoding? I would just use small, large, huge names consistently.

In addition to Paul's comment, the `getInt` test doesn't seem 100% appropriate, since `sumInt` is using `C_INT` but
`getInt` is using `C_POINTER` - maybe rename to getPointer? Overall the test is very comprehensive and I think it will
spare us a lot of bugs going forwards, thanks.

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

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


More information about the panama-dev mailing list