[foreign] RFR: Port clang-ffi source code to new API

[Maurizio Cimadamore]

I like it - it goes in the right direction of connecting the ABI and 
layouts more closely together.

The only bit I don't get is the one on undefined struct - you changed 
some tests, but you seem to suggest that jextract is not generating an 
annotation for undefined structs, but that's not the case?

E.g. if I jextract the following header:

//testUndefined.h

typedef struct UndefinedStruct UndefinedStruct;

int f(struct Foo *p);


I get back two classes:

testUndefined.class

testUndefined$UndefinedStruct.class

The latter is an annotation for the typedef. So I guess I don't get what 
you mean when you say " modify test to reflect the removal annotation of 
undefined struct" - can you please expand on that a little?

Thanks!
Maurizio


On 01/06/18 06:08, Henry Jen wrote:
>> On May 31, 2018, at 7:56 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>
>> Some comments
>>
>> * to instantiate the SystemABI in a platform agnostic way, see the static factory SystemABI.getInstance (right now it just creates SysV ABI, but I think you don't want to call 'new' on that yourself)
>>
>> * I think you can remove some cruft for Void handling, now that I've pushed the related patch
>>
> Thanks for the review, I have updated the webrev[1] to reflect above two.
>   
>> * The changes in ABI go in the opposite direction of what I'd like to see - e.g. I'd like ABI impl to be layout-centric, not CType centric. So, I'm ok with having some kind of enum for common c types, but I think this enum should give you back the corresponding layout under given ABI - e.g.
>>
>> Layout layoutFor(CType) { …
>> }
>>
> Fair enough, adapted in the webrev.
>
>> And leave definedAlignment and definedSize alone. This will allow you to reduce changes in the System ABI classes.
>>
> The purpose of those two methods are to know the right size and alignment requirement for a particular type. My understanding to Layout is that it suppose to eliminate all platform uncertainty, meaning the result should be constant, we don’t really need those two methods. So I removed them. sizeof and alignment methods should be done based on the size of Layout.
>
> Make sense?
>
> In this updated webrev, I also add null pointer test and modify test to reflect the removal annotation of undefined struct. As undefined struct is used by Pointer only, and we always return Pointer<Void>, without the annotation is no big deal just that we have no hint at all. But I understand this is still an open discussion on how to deal with it.
>
> [1] http://cr.openjdk.java.net/~henryjen/panama/foreign/jclang-ffi/1/webrev/
>
> Cheers,
> Henry