BUG: function pointers inside a struct do not generate SegmentAllocators when needed

[Marko]

My apologies, gmail seems to have only replied directly to Jorn
previously. Here is the original message.

Hey Jorn,
Thanks for filing the report. I agree that the problem is where the
`ofAddress` function is generated. It does not check to see if a
segment allocator is needed. It seems quite trivial to add though as
it is already implemented for regular functions,
https://github.com/openjdk/jextract/blob/0582eaf1b4cdba95f0ee8c2480767433bb647d0d/src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java#L122-L127.

However, when this is fixed, the getter must also be updated to
reflect the addition of the SegmentAllocator parameter.
I believe this constitutes an additional parameter here
https://github.com/openjdk/jextract/blob/0582eaf1b4cdba95f0ee8c2480767433bb647d0d/src/main/java/org/openjdk/jextract/impl/StructBuilder.java#L128=
similar to fiName, that represents whether a segment allocator is
needed or not. It seems a bit odd though IMHO, as then
HeaderFileBuilder#addVar and ToplevelBuilder#addVar are also forced to
have this parameter even though it goes unused there.

I implemented the fix and the proper getter generation as well, and it
seems to all work on my end. I wonder if this boolean parameter is the
way to go before submitting a PR though.

Thanks,
Marko

On Thu, Jul 28, 2022 at 5:20 PM Jorn Vernee <jorn.vernee at oracle.com> wrote:
>
> Hello Marko,
>
> Thanks for the report! I can reproduce it here as well. I think the
> problem is the lambda in the `ofAddress` factory that is generated in
> the interface for the callback type. Note that the getter will just end
> up calling that factory. (you can see that also in your stack trace: `at
> Foo$bar.lambda$ofAddress$0(Foo.java:32)`).
>
> I've filed: https://bugs.openjdk.org/browse/CODETOOLS-7903239
>
> Cheers,
> Jorn
>
> On 28/07/2022 02:00, Marko wrote:
> > Hello again!
> > When trying to call a function pointer inside a struct which returns a
> > MemorySegment, I receive the following error:
> > ```
> > Exception in thread "main" java.lang.AssertionError: should not reach here
> > at Foo$bar.lambda$ofAddress$0(Foo.java:34)
> > at Main.main(Main.java:8)
> > Caused by: java.lang.invoke.WrongMethodTypeException: expected
> > (NativeSymbol,SegmentAllocator)MemorySegment but found
> > (NativeSymbol)MemorySegment
> > at java.base/java.lang.invoke.Invokers.newWrongMethodTypeException(Invokers.java:523)
> > at java.base/java.lang.invoke.Invokers.checkExactType(Invokers.java:532)
> > at Foo$bar.lambda$ofAddress$0(Foo.java:32)
> > ... 1 more
> > ```
> > I believe this is due to
> > `FunctionalInterfaceBuilder#emitFunctionalFactoryForPointer` not
> > checking to see if the returnType is a MemorySegment, as it does in
> > `HeaderFileBuilder` for regular functions, see
> > https://github.com/openjdk/jextract/blob/0582eaf1b4cdba95f0ee8c2480767433bb647d0d/src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java#L122-L127.
> >
> > I began implementing a fix by copying this functionality over, however
> > there is a bit of an issue I came across. When jextract emits the
> > getter for the functional interface in a struct (StructBuilder), only
> > the name of the interface is known. AFAIK, it does not know whether it
> > needs a SegmentAllocator and cannot currently generate the correct
> > code.
> >
> > I thought about adding another field to the method
> > `emitFunctionalFactoryForPointer`, similar to fiName, like a boolean
> > for whether a segment allocator is needed or not, however, I can't help but
> > feel it is a bit hacky. I wonder what your opinions are on this.
> >
> > A repro can be found here:
> > https://github.com/lost-illusi0n/jextract-func-ptr-seg-alloc-repro
> > Just run the run script if you are on Linux. You will have to change the
> > library output in the script if you are on another os.