[foreign-jextract] RFR: 8252759: Jextract flattens nested anonymous structs and unions when computing layouts

[Maurizio Cimadamore]

On Thu, 14 Jan 2021 18:54:43 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> Hi,
> 
> This patch changes the way jextract handles nested structs and unions.
> 
> Currently, when encountering an anonymous struct or union, jextract folds it's fields into the parent layout. But, this can lead to incorrect layouts, especially for unions. e.g. see the following example:
> 
>     struct Foo {
>         int x;
>         union {
>             int y;
>             int z;
>         };
>     }; 
>     
> becomes:
> 
>     static final MemoryLayout Foo$struct$LAYOUT_ = MemoryLayout.ofStruct(
>         C_INT.withName("x"),
>         C_INT.withName("y"),
>         C_INT.withName("z")
>     ).withName("Foo"); 
>     
> In this case, the offset of the `z` field will not be correct.
> 
> This patch changes the jextract behaviour to preserve the nested layouts of anonymous unions and structs, so the example becomes:
> 
>     static final MemoryLayout Foo$struct$LAYOUT_ = MemoryLayout.ofStruct(
>         C_INT.withName("x"),
>         MemoryLayout.ofUnion(
>             C_INT.withName("y"),
>             C_INT.withName("z")
>         ).withName("$anon$0")
>     ).withName("Foo"); 
>     
> Here, the anonymous layout gets a generated name which is needed so that the `y` and `z` fields can still be reached from the root layout with layout paths to create var handles for instance.
> 
> Just the layout is changed, the Foo java class that we generate still has getters and setters for `y` and `z`.
> 
> In the patch this behaviour is implemented by temporarily adding a 'prefix' (`"$anon$0"` in this case) to the `StructBuilder`, which is used in the generated layout paths to reach the nested fields.
> 
> Thanks,
> Jorn

I like the approach, because not only it fixes code generation, but it also makes the AST "right" by avoiding early flattening. I think the test is a bit light - we might want to add more cases of deeply nested anon structs, just in case - as this area is a bit fragile.

src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/RecordLayoutComputer.java line 108:

> 106: 
> 107:     void addFieldLayout(long offset, Type parent, Cursor c) {
> 108:         MemoryLayout memoryLayout = c.isAnonymousStruct()

So, am I right that this avoid flattening of the anon structs fields into enclosing layout - e.g. layouts coming out of the parser now are non-flattened, but we tag them with an attribute so that we can emit flattened accessors later?

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

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