[foreign-abi] RFR: Revisit FunctionDescriptor

[Jorn Vernee]

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
>>