[foreign-jextract] RFR: 8261906: Improve jextract support for virtual functions [v6]

[Jorn Vernee]

On Thu, 4 Mar 2021 00:11:56 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This patch add support for function pointer calls. Whenever jextract encounters a function pointer variable (be it a global variable, or a struct field), it will additionally emit code to _call_ the function pointer - by means of a static wrapper around a virtual method handle.
>> 
>> The implementation is relatively straightforward, although there is some duplication in the code that is being emitted. I played with this a bit, to see if duplication could be removed, but I ended up with a single routine to generate wrappers which worked across the axis { virtual, non virtual } x { struct, global } - which in the end I found too complex for my taste (note that, e.g. when we're inside a struct, the logic to get the virtual address depends on what else is generated inside the struct, so there's a lot of ad-hocness).
>> 
>> For these reason I ended up with separate emit function for virtual wrappers (e.g. I did not reuse the code for native functions) - the code which emit virtual function wrappers is also overridden in StructBuilder. My sense is that if we move the code to use text blocks, the duplication will be much less problematic.
>> 
>> In OutputFactory, I cleaned up things a bit - since I realized that support for valist doesn't work - this is now removed. I will file a followup issue to add proper VaList support, both in downcalls and upcalls.
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add call to Utils.javaSafeIdentifier

Changes look good! Though there is a merge failure it looks like.

src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/FunctionalInterfaceBuilder.java line 54:

> 52:         emitFunctionalInterfaceMethod();
> 53:         emitFunctionalFactories();
> 54:         emitFunctionaRestrictedFactory();

Typo
Suggestion:

        emitFunctionalRestrictedFactory();

src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/OutputFactory.java line 264:

> 262:                 Type.Function f = getAsFunctionPointer(param.type());
> 263:                 if (f != null) {
> 264:                     String name = funcTree.name() + "$" + (param.name().isEmpty() ? "x" + i : param.name());

This seems to let the name of the functional interface class depend on the parameter name? I thought we did _not_ want to do that?

src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/OutputFactory.java line 408:

> 406:                 varInfo = VarInfo.ofFunctionalPointerVar(clazz, layout, Utils.javaSafeIdentifier(funcTypedef.get()));
> 407:             }
> 408:         }

I see what you mean here. I guess we expect these functional interface classes to be generated when visiting the typedef?

test/jdk/tools/jextract/funcPointerInvokers/TestFuncPointerInvokers.java line 54:

> 52:             MemorySegment bar = Bar.allocate(scope);
> 53:             Bar.foo$set(bar, Foo.allocate((i) -> val.set(i), scope).address());
> 54:             Bar.foo(bar).apply(42);

Cool! :)

-------------

Marked as reviewed by jvernee (Committer).

PR: https://git.openjdk.java.net/panama-foreign/pull/456