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

Maurizio Cimadamore mcimadamore at openjdk.java.net
Fri Jan 15 17:15:10 UTC 2021


On Fri, 15 Jan 2021 14:38:36 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
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Unify tests

Looks great! Thanks for making the test more portable too (e.g. `long long`).

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

Marked as reviewed by mcimadamore (Committer).

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


More information about the panama-dev mailing list