[foreign] RFC 8219042: Cross-header layout resolution does not work

[Maurizio Cimadamore]

On 15/02/2019 15:15, Jorn Vernee wrote:
> I think we can say that if a user wants to hand write header 
> interfaces, then it's also up to them to make sure that all layouts 
> can be resolved. I think we could also help them by documenting the 
> lookup process carefully.
>
> 1. sounds like a good option to me, but I'm wondering if this approach 
> will run into class file limitations for very large libraries (like 
> Windows.h)?
Note that this doesn't mean generating only one class - there can be 
many _nested_ classes, but only one root. So unless you run out of 
methods, I don't think classfile size will be a concern.
>
> Having the ability to manually map names to classes like you suggest 
> in 2. seems a bit overkill to me. Maybe as an intermediate solution we 
> could change the Unresolved layout descriptors to denote Java 
> signatures, instead of layout names. e.g 
> '@NativeFunction("${org.package.MyClass::func}")', similarly for classes.

I've been looking at similar ideas in the past - but ended up discarding 
them because they end up being messy. I think it's nice to keep the 
layouts Java free - and also, stringifying a class has issues - because 
then you have to infer which class loader should be used, and that is 
also messy.


As for (2), I think there are many intermediate steps we could take - 
for instance, instead of explicit name -> class mapping, we could simply 
define a list of 'friends' toplevel classes the resolver has to scan.

So, if you are in header A and you refer to something in header B, you 
can say @Friend(B.class), for example.

This will still be miles better than scanning all signatures in the hope 
for a match.

Maurizio


> Then we could rely on Java resolution mechanisms.
>
> ---
>
> About the patch;
>
> - ScanType still has code to deal with Java arrays, is this still 
> needed? I think that's left from before Array was introduced? Maybe 
> now is a good moment to remove it.
>
> - LayoutResolver::scanClassInternal also does a check:
>
>     if (nameToGroup.containsKey(name) && nameToGroup.get(name) != l) {
>         throw new IllegalArgumentException(name + " cannot be 
> redefined");
>     }
>
> But I believe that should use `!l.equals(nameToGroup.get(name))` since 
> `l` is a freshly created Layout Object.
>
> Jorn
>
> Maurizio Cimadamore schreef op 2019-02-15 14:51:
>> Hi,
>> the following patch is inspired from the work that Jorn posted
>> yesterday [1]. But it takes it a step further: instead of having the
>> various code generators call 'scanMethod' and such, all 'indexing'
>> activity is done before hand, when te resolver is first created.
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8219042/
>>
>> Now, as pointed out, this approach is very ad-hoc; not only it is
>> inefficient (it's doing a full reflective scan of all methods in all
>> classes of an extracted artifact!), but it's also fragile, as it
>> relies on the Java signature to give away some info as to what a
>> layout name might resolve to. Consider the following case:
>>
>> @NativeHeader
>> interface MyHeader {
>>
>> @NativeFunction("()u64:${foo}")
>> Pointer<?> m();
>>
>> }
>>
>> In this case, despite there's an entry point for 'm', the signature
>> info associated with such an entry is simply insufficient, as there's
>> no way to tell which carrier type is associated with the name 'foo'.
>>
>> It seems to me that there are only two stable solutions in this space:
>>
>> 1) assuming jextract generate an artifact with a single root, we could
>> then index all inner classes under that root, to look for structs
>> (this depends on the jextract library-centric work)
>>
>> 2) if we don't want to make assumption on the layout of the extracted
>> interfaces (which seems hard, especially when considering hand-written
>> interfaces), then the only option is to manually define a resolution
>> context:
>>
>>
>> @NativeHeader
>> @NameContext(@NameMapping("foo", Foo.class))
>>
>> interface MyHeader {
>>
>> @NativeFunction("()u64:${foo}")
>> Pointer<?> m();
>>
>> }
>>
>>
>> (name and shape of the annotations TBD).
>>
>> This way, when the binder creates a resolver, it sets up all the
>> necessary mapping from names to layouts. The binder should also check
>> the validity of the mappings (e.g. check that Foo.class is a struct
>> and that it really defines a layout with name 'foo').
>>
>> This way, no guesswork is needed, everything is deterministic and
>> efficient. And this scales to the context of manually written mutually
>> dependent structs - which is probably not an uncommon case when it
>> comes to message protocols (no jextract there).
>>
>> Thoughts?
>>
>> Maurizio