[foreign-jextract] RFR: 8251256: jextrac code generated for nested structs, unions is incorrect

[Henry Jen]

On Thu, 6 Aug 2020 17:39:46 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This patch rectifies the behavior of jextract with respect to nested structs and/or unions.
> 
> More specifically, jextract failed to recursively generate interfaces for anonymous structs and unioins; this led to a
> situation (as described in the JBS issue) where the only operation a client can do is to retrieve a segment for the
> nested struct/union, but all nested accessors are not available.  The fix allows jextract to recurse; more
> specifically, if a struct/union field type is itself an anonymous struct/union, the interface for the anonymous
> struct/union will be generated (nested inside the current struct interface).  So, in the example submitted in the JBS
> issue, it would be possible to poke at the nested union field, as follows:
>  MemorySegment rdma = ibv_send_wr.wr$addr(someSegment);
>  ibv_send_wr.wr.rdma.remote_addr$get(rdma);
> 
> Note the nesting: `ibv_send_wr.wr.rdma` where:
> 
> * `ibv_send_wr` is the enclosing struct declaration
> * `wr` is the union declaration, nested inside the struct
> * `rdma` is a struct that is one of the union alternatives
> 
> To make the code more uniform, I've reworked JavaSourceBuilder a little - more specifically:
> 
> * JavaSourceBuilder is aware of the fact that builders can *nest* into each other
> * Rather than dumping contents of a child builder into the parent, we use delegation, to make sure that all calls to
>   `append` eventually hit the `StringBuilder` in the toplevel builder --- effectively removing the need to copy
> * each builder has its own name context (to fixup duplicate names); this follows from the fact that each nested class
>   introduces a fresh scope, so no clashes with names defined outside are possible
> 
> While I've added no test, I found out that an existing test (Test8244512) was already triggering the scenario described
> in the JBS issue. I've tweaked the test to take into account the fact that now jextract correctly nests inner structs,
> as well as the fact that jextract is now adding interfaces even for the anon structs/union attached to struct/union
> fields.

Overall looks good, I tested it on MacOS and seems like we get pretty good coverage. Some minor issue with indentation
of generated code, but that's minor and can be fixed later. The only case missed seems to be when an anonymous type  in
a function argument like following, =c void foo(struct {int x; int y; } *p);
I doubt that's important as while it's legal, sane programmer should not code like that as the struct type is not
exposed. So it seems fit jextract not expose the layout either.

FWIW, there is a header file I used for testing anonymous types and dependency, the result from jextract did cover all
anonymous types. There are still something both jbind/jextract cannot handle, but that's beyond this issue.

https://github.com/slowhog/panama-foreign/blob/jbind/test/jdk/java/jextract/anonymousDecl.h

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

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