[foreign] rethinking Panama annotations

[Maurizio Cimadamore]

I also put together a patch which more or less implements this:

http://cr.openjdk.java.net/~mcimadamore/panama/unresolved%2bcallback_v3/

It works, in the sense that I've been able to rewrite most of the tests 
to use the more compact by-name notation.

But there are some hiccups: first I don't know exactly how to change 
jextract to generate by-name references for function pointers, given 
that, currently, function pointer interface names are not derived from 
the signature (e.g. their name is FI<N> where N is a number which 
essentially tells you at which point jextract did find the interface).

This patch also re-uncovered some of the issues around unresolved 
layouts that we know of - for instance if you try to do a 
Scope::allocateStruct of a struct that has some unresolved fields - e.g.

[ $(foo) i32 ]

The allocation will fail because the Scope::allocate method will not 
attempt layout resolution. And, if we change things so that it performs 
layout resolution, we run into other issues (different layout 'names' 
introduced in different header files sometimes are not equally 
resolvable from all possible extracted classes). These issues are a 
result of the fact that jextract models separate headers with separate 
interfaces (even when one included the other). We have already discussed 
how jextract ought to adopt a more 'flattened' approach, where 1 
interface ~ 1 library (which can be defined by N headers).

To workaround this limitation I put in some hacks around 
LayoutType::ofStruct which are enough to keep the tests happy.

But, stepping back, the takeaway is that, the general approach is indeed 
possible, but we need to fix some other issues around name resolution 
before we can really tackle this problem head on.

Maurizio


On 26/11/2018 21:07, Maurizio Cimadamore wrote:
> Good catch, I fixed the table (again :-))
>
> Maurizio
>
> On 26/11/2018 19:57, Jorn Vernee wrote:
>> Looking at the new table, I see now that the annotation is on the 
>> function descriptor and not on the address layout for the 
>> Layout-Based approach as well. This seems strange to me since the 
>> layout hole `@{qsortComp}` will be replaced by `u64:(i32 i32)i32`, 
>> not just by `(i32 i32)i32`, so I'd expect the name annotation to be 
>> on the address layout, not on the function descriptor.
>>
>> This is why I was confused that the declaration for the layout based 
>> approach didn't parse for me; I thought the name annotation was 
>> supposed to be on the layout.
>>
>> Jorn
>>
>> Maurizio Cimadamore schreef op 2018-11-26 19:45:
>>> On 26/11/2018 18:42, Jorn Vernee wrote:
>>>> This seems fine to me, after all Callbacks are a carrier for 
>>>> function pointers, so @NativeCallback declaring the layout of a 
>>>> function pointer seems to make sense from a user perspective as well.
>>>>
>>>> Just so we're clear, wouldn't the use site of Layout-Based actually 
>>>> be `qsort=(u64: i32 i32 ${qsortComp})v` instead of `qsort=(u64: i32 
>>>> i32 u64:${qsortComp})v` (so without the u64:)?
>>>>
>>> Yep - cut and paste issue, corrected here
>>>
>>> http://cr.openjdk.java.net/~mcimadamore/panama/callback-table.html
>>>
>>>> Also, `Layout.of("u64:(i32 i32)(qsortComp)i32")` is not parsing for 
>>>> me, I have to use `Layout.of("u64(qsortComp):(i32 i32)i32")`, and 
>>>> then the name correctly appears on the returned layout as well.
>>>
>>> That depends on the patch submitted for RFR this morning, see [2]
>>>
>>> [2] 
>>> http://mail.openjdk.java.net/pipermail/panama-dev/2018-November/003287.html
>>>
>>>>
>>>> Jorn
>>>>
>>>> Maurizio Cimadamore schreef op 2018-11-26 18:56:
>>>>> On 26/11/2018 14:15, Maurizio Cimadamore wrote:
>>>>>
>>>>>> separate patch for adding support for descriptor name resolution in
>>>>>> callbacks
>>>>>
>>>>> I've been thinking more about this, and it's not as easy as it looks
>>>>> at first...
>>>>>
>>>>> The issue is that if we want to support resolution for both struct
>>>>> names and callback names, that means that Unresolved essentially
>>>>> becomes a 'descriptor', not a layout. But if we pull on this string
>>>>> more, then we need to replace descriptor for layout everywhere in the
>>>>> API, de facto flattening the API (e.g. clients will be using
>>>>> descriptor everywhere, thus making the descriptor vs. layout split
>>>>> useless).
>>>>>
>>>>> The only way I see to support name resolution for callbacks and, at
>>>>> the same time, retain the descriptor vs. layout split, is to tweak 
>>>>> the
>>>>> @NativeCallback annotation so that its value attribute describes a
>>>>> full address layout, not just the signature of the callback - e.g.
>>>>>
>>>>>  Vanilla
>>>>>  Descriptor-based
>>>>>  Layout-based
>>>>>
>>>>>  Declaration
>>>>>  @NativeCallback("(i32 i32)i32")
>>>>> interface QsortComp { ... } @NativeCallback("(i32 
>>>>> i32)(qsortComp)i32")
>>>>> interface QsortComp { ... } @NativeCallback("u64:(i32
>>>>> i32)(qsortComp)i32")
>>>>> interface QsortComp { ... }
>>>>>
>>>>>  Use-site
>>>>>  qsort=(u64:[0i32]i32i32u64:(u64:i32u64:i32)i32)v
>>>>>          qsort=(u64: i32 i32 u64:${qsortComp})v
>>>>>          qsort=(u64: i32 i32 u64:${qsortComp})v
>>>>>
>>>>> So, first notice how much compaction we get by being able to refer to
>>>>> callbacks by name - the use site values in the second and third 
>>>>> column
>>>>> are clearly more legible than the one in the first column, in which
>>>>> tne entire callback signature has to be repeated (which is also error
>>>>> prone, if you write these things by hand).
>>>>>
>>>>> The difference between descriptor-based and layout-based is that in
>>>>> the former, we are able to refer to a function descriptor directly
>>>>> using an unresolved hole; in the latter case, the unresolved hole 
>>>>> will
>>>>> resolve to a layout (an address layout whose pointee is a function).
>>>>> Therefore, the layout-based approach preserves all the properties we
>>>>> care about: it allows to retain the distinction between layouts and
>>>>> functions, while at the same time allowing for some for of by-name
>>>>> reference for callbacks. The small price to pay is that the contents
>>>>> of the @NativeCallback annotation will now contain not just the
>>>>> callback signature, but the full address layout corresponding to a
>>>>> function pointer associated to that callback.
>>>>>
>>>>> Is this something that we consider acceptable?
>>>>>
>>>>> Cheers
>>>>> Maurizio