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

[Marko]

Perhaps the additional parameter was not as weird as I originally thought.
> 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 just tried it out and the getters are also missing when generated as
a part of the header file.
I'll polish my fix and send a PR for a better opinion.

Thanks
Marko


On Thu, Jul 28, 2022 at 7:25 PM Marko <im at lostillusion.net> wrote:
>
> 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.