[foreign] RFR 8221471: jextract source genarator generates non-compilable classes with naming clashes

Sundararajan Athijegannathan sundararajan.athijegannathan at oracle.com
Wed Apr 10 03:01:43 UTC 2019

Thanks for catching these...

Updated webrev: 

Responses to your review comments/test observations below..

On 09/04/19, 11:07 PM, Jorn Vernee wrote:
> Windows has some failures, seemingly from to va_list patch.
> 1.) small typo in IncompleArrayTest (for the Windows-only case):
> -                
> assertNotNull(loader.loadClass(headerInterfaceName("incompleteArray3i.h")));
> +                
> assertNotNull(loader.loadClass(headerInterfaceName("incompleteArray3.h")));

The va_list patch does not seem to have touched this file.


Damage has been done by even earlier patch!


My mistake! I should have all "all platforms" tests. Thanks for catching 

> 2.) ValistUseTest itself fails. It looks like the builtin$.h header is 
> missing from src/jdk.jextract/share/conf for some reason [1], I guess 
> it got dropped from the push by accident? But, after I added that the 
> test still failed.

I missed pushing builtin$.h as part of the va_list patch :( Added it in 
the updated webrev above.

> I manually ran jextract on the test header, which works. The generated 
> method uses Pointer<Byte> for the va_list, so it looks like the 
> backing impl is different on Windows, and there's no need for jextract 
> to generate the __va_list_tag struct in clang_support.builtin$_h that 
> the test is checking for, so the test fails.
> Could go with adding `@requires os.family != "windows"` to 
> ValistUseTest for that.

As suggested, I made this test platform specific - va_list handling is 
indeed platform specific.

> After those 2 fixes the tests are all green on my machine :)
Thanks for testing on Windows! [and welcome back! :) ]

My mach5 "all platforms testing" with updated patch is successful as well.


> Cheers,
> Jorn
> [1] : 
> http://hg.openjdk.java.net/panama/dev/file/d6f2bf38b95b/src/jdk.jextract/share
> Maurizio Cimadamore schreef op 2019-04-09 18:20:
>> Looks good!
>> A multi-platform test run is probably better here :-)
>> Maurizio
>> On 09/04/2019 16:55, Sundararajan Athijegannathan wrote:
>>> Please review.
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8221471
>>> Webrev: 
>>> https://cr.openjdk.java.net/~sundar/8221471/webrev.02/index.html
>>> Current naming scheme: Header interfaces are named as "foo". Static 
>>> forwarder class is named as "foo_h"
>>> New naming scheme: Header interfaces are named as "foo_h". Static 
>>> forwarder class is named as "foo_lib"
>>> Thanks,
>>> -Sundar

More information about the panama-dev mailing list