[foreign] RFR 8220063: Generate LayouType constants as part of jextract output

[Maurizio Cimadamore]

On 05/03/2019 17:53, Jorn Vernee wrote:
>> My gripe here is: where is the 'size' of the address coming from? I
>> see an hardwired '64' here?
>
> The Address layout is coming from 3 places, and all use a fixed 64 
> size. 2 are in the PointerScopeTest, which I've removed. The other 
> comes (AFAICT) from jextract in LayoutUtils:
>
> ```
> case FunctionProto:
>     return Address.ofFunction(64, parseFunctionInternal(t));
> ```
>
> Like you say, it's a hack, but I've only moved it around, not created it.
Sure, but... now there's no way to create a callback with an address 
other than 64 in the public API; not a big issue for now, but...
>
> I was thinking of adding a SystemABI.addressSize() or 
> Address.platfromSize() or something like that to use instead of the 
> hardcoded '64'.

I think a layout encodes information about:

* size (now)
* alignment (in the future)
* endianness (also now)

You can infer something that 'makes' sense for most of these, given the 
platform at hand; but I believe that an API that takes an explicit 
layout will always be more scalable than an API with a bunch of 
hardwired call.

What I'm saying is not opposed to what you are saying - we can have our 
cake and eat it too: just have your Address-less version be an overload 
and call the Address-ful version!

Maurizio

>
> What do you think?
>
> Jorn
>
> Maurizio Cimadamore schreef op 2019-03-05 18:39:
>> On 05/03/2019 16:16, Jorn Vernee wrote:
>>> Maurizio Cimadamore schreef op 2019-03-05 17:06:
>>>> On top of my head, this looks something more for the Ext code factory
>>>> than for the main one.
>>>
>>> But the Ext code factory is for generating the static forwarder. I 
>>> added it to AsmCodeFactory since the changes affect the classes in 
>>> the regular (non-"_h") classes.
>>
>> Good point - doing this kind of things in code factory EXT might be
>> tricky - you might want to sync up with Sundar who's looking at how to
>> group enum constants in interfaces in the EXT backend:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8219821
>>
>>
>>>
>>> If you think this feels more like an extension, maybe we should try 
>>> to figure out some extension/plugin framework first, and then 
>>> implement this feature on top of that (would be a good test case).
>>>
>>>> Also, not clear about the changes in LayoutType.ofFunction... there's
>>>> no inference going on whatsoever, in fact your patch does this
>>>>
>>>> +        return LayoutTypeImpl.ofCallback(Address.ofFunction(64,
>>>> Util.functionof(funcIntf)), References.ofFunction, funcIntf);
>>>>
>>>> Yes, we discussed about changing @NativeCallback while back, but I'm
>>>> not 100% sure that's the direction we should go, and I'd like not to
>>>> couple things to it.
>>>
>>> Instead of the Address layout being passed in, the Function layout 
>>> is extracted from the @NativeCallback annotation. That's what I mean 
>>> by "infer that from the @NativeCallback".
>>
>> My gripe here is: where is the 'size' of the address coming from? I
>> see an hardwired '64' here?
>>
>> Maurizio
>>
>>>
>>> I'm not sure what you mean about changing @NativeCallback? This 
>>> change relies on the current implementation.
>>>
>>> Jorn
>>>
>>>> Maurizio
>>>> On 05/03/2019 15:41, Jorn Vernee wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I'd like to contribute a patch which let's jextract generate
>>>>> LayoutType constants in typedef annotation classes, and struct
>>>>> classes for the underlying type. The LayoutType constant has the
>>>>> name of the typedef or struct with an "_t" suffix.
>>>>>
>>>>> This is useful because;
>>>>>
>>>>> 1.) No need to manually declare a static final LayoutType if you
>>>>> don't want to call LayoutType.ofStruct(...) everywhere.
>>>>>
>>>>> 2.) If an API uses a type in it's documentation/tutorials, but that
>>>>> type comes from a typedef, in the generated Jextract bindings you
>>>>> will just find the typedef annotation with that name. But, if we add
>>>>> a LayoutType constant to the typedef annotation class, the user has
>>>>> an easy way to find the underlying type as well. (Very useful with
>>>>> APIs that use typedefs for pretty much anything. Like the Windows
>>>>> API).
>>>>>
>>>>> I might be a little biased towards this, but I've found that
>>>>> declaring static final LayoutType constants is a must have in
>>>>> non-trivial projects. Both to reduce verbosity and to show the name
>>>>> of the API type in the code, not just the name of the underlying
>>>>> type. e.g. `scope.allocate(MyType_t)` vs.
>>>>> `scope.allocate(DOUBLE.array(2))`.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8220063
>>>>> Webrev:
>>>>>
>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220063/webrev.00/
>>>>>
>>>>>
>>>>> I did 3 other things to implement this:
>>>>>
>>>>> 1.) Drop Address parameter of LayoutType.ofFunction, since we can
>>>>> just infer that from the @NativeCallback annotation instead.
>>>>> 2.) Fixed issue where a typedef of a function type was not
>>>>> generating an @NativeCallback class.
>>>>> 3.) Sharpened typing in TypeDictionary and JType w.r.t.
>>>>> JType.FunctionalInterfaceType.
>>>>>
>>>>> Thanks,
>>>>> Jorn