[foreign-abi] RFR: Revisit FunctionDescriptor

[Maurizio Cimadamore]

What about having two factory methods (as we have in other layout 
classes) - e.g.

FunctionDescriptor.of( ... )
FunctionDescriptor.ofVariadic(...)

Maurizio

On 04/02/2020 10:14, Jorn Vernee wrote:
> Ah, I missed the use in RuntimeHelper. That seems to be the only real 
> use case.
>
> Wrt using annotations instead, I guess what I'm saying is; I don't see 
> that much value in being able to attach this single bit of information 
> to a FunctionDescriptor, but I see more value in being able to attach 
> any kind of information to a FunctionDescriptor. Since the ABI 
> machinery doesn't use it either, the variadic flag seems like an 
> arbitrary appendage. Something like that feels more at home as an 
> annotation (a mechanism for arbitrary appendages).
>
> Though, my main gripe is with having to read (and write) the extra 
> boolean argument all the time when a FunctionDescriptor is created, 
> even though this extra visual information is rarely relevant (and 
> indeed, it is irrelevant if you're reading just foreign-abi code). 
> That's the main thing I'd like to remove. Since the flag is currently 
> being used, I'd suggest replacing the boolean flag passed to the 
> factory with an asVariadic() method.
>
> New webrev: 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/cleanup_fd/webrev.01/
>
> Thoughts?
>
> Jorn
>
> On 03/02/2020 19:10, Maurizio Cimadamore wrote:
>> (without having looked at the code) - jextract runtime support is 
>> currently using the varargs info to generate varargs MethodHandle 
>> which do the specialization. As you say, we can tackle that problem 
>> with annotations, although I'm not sure of what's the net gain in 
>> moving things around. At this point I guess I'd prefer jextract to 
>> not generate any magic annotation - the Declaration.Function class 
>> knows whether a function is a varargs or not, so there's no need to 
>> truck that info into the runtime.
>>
>> Maurizio
>>
>> On 03/02/2020 16:19, Jorn Vernee wrote:
>>> Hi,
>>>
>>> Please review the following patch that revisits FunctionDescriptor: 
>>> 1. Adds appendArgumentLayouts, and changeReturnLayout combinator 
>>> methods. 2. Removes the variadic flag. This flag was not really 
>>> being used in practice, since we can not link variadic functions 
>>> directly any ways. Using the new appendArgumentLayouts method we can 
>>> 'specialize' a base FD with different argument layouts representing 
>>> vararg arguments. I think if a similar flag is needed in the future, 
>>> we should consider adding annotations to FunctionDescriptor, and 
>>> making it an annotation instead, but currently it doesn't seem like 
>>> it's pulling it's weight by being in the API.
>>>
>>> Webrev: 
>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/cleanup_fd/webrev.00/ 
>>>
>>> Bugs: https://bugs.openjdk.java.net/browse/JDK-8238226, 
>>> https://bugs.openjdk.java.net/browse/JDK-8237580
>>>
>>> Again, this patch applies on top of the Binding cleanup patch.
>>>
>>> Thanks,
>>> Jorn
>>>