[foreign] RFR 8217664: jextract should check for error code returns from Type::getOffsetOf and Type::size

[Sundararajan Athijegannathan]

Unused code should be removed and not just commented (struct.h for eg). 
We've history in the repo and we can always go back.

PS. I'll build/test on Mac and respond.

-Sundar

On 25/01/19, 1:10 AM, Jorn Vernee wrote:
> Updated webrev for recent push: 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8217664/webrev.03/
>
> Jorn
>
> Jorn Vernee schreef op 2019-01-24 14:04:
>> Hi,
>>
>> jextract uses libclang functions to query the size of a type and
>> offset of a field. The returned values are used for instance in layout
>> calculations. But, these functions can also return error codes and
>> there is currently no check for them. They are happily being used in
>> the layout calculations, and this can cause some strange errors, or in
>> some cases incorrect layouts are generated. To give an example:
>>
>>     struct Foo {
>>         int x;
>>         void *list_of_data[];
>>     };
>>
>> The expected layout of this struct (which uses an incomplete/flexible
>> array field) should be `[i32x32[0u64]]` but jextract instead generates
>> `[i32[0u64]x32]`. This particular case is due to a bug in libclang
>> where requesting the offset of a field in a type with a flexible array
>> member always returns an incomplete type error, even though getting
>> the offset of each fields should be possible. This error code (-2) is
>> then used in the layout calculations and yields the incorrect result.
>>
>> So I've added checking to the returned value of Type::size and
>> Type::getOffsetOf and throwing an exception in case an error code is
>> returned. I'm also explicitly checking for the flexible array case and
>> throwing a UOE in that case. This also means I had to disable the
>> flexible array jextract test [1]. The test was passing, but at least
>> on Windows the wrong layout was being generated. Since we can not
>> reliably compute padding for types with flexible arrays using the
>> current method, I think it's better to throw an exception for now
>> until we can find a workaround for this bug in libclang, or the bug
>> gets fixed.
>>
>> Using a type with a flexible/incomplete array member in the binder
>> should still be fine of course, so I've added a separate test for
>> that.
>>
>> Please review the following.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8217664
>> Webrev: 
>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8217664/webrev.00/
>>
>> Thanks,
>> Jorn
>>
>> [1] :
>> http://hg.openjdk.java.net/panama/dev/file/c40a5a6dc24d/test/jdk/com/sun/tools/jextract/testStruct/struct.h#l85 
>>