RFR: 7903649: Field and global variables of array type should have indexed accessors [v3]

[Jorn Vernee]

On Wed, 31 Jan 2024 18:20:37 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This PR adds support for indexed accessors for struct fields and global variables whose type is an array type. These accessors feature a number of `long` access coordinates which has the same cardinality as that of the underlying array type.
>> 
>> For instance, consider the following global variable declaration:
>> 
>> 
>> int ints[2][3][4];
>> 
>> 
>> For this, jextract now emits:
>> 
>> 
>>     private static SequenceLayout ints$LAYOUT() {
>>         class Holder {
>>             static final SequenceLayout LAYOUT = MemoryLayout.sequenceLayout(2, MemoryLayout.sequenceLayout(3, MemoryLayout.sequenceLayout(4, foo_h.C_INT)));
>>         }
>>         return Holder.LAYOUT;
>>     }
>> 
>>     private static MemorySegment ints$SEGMENT() {
>>         class Holder {
>>             static final MemorySegment SEGMENT = foo_h.findOrThrow("ints")
>>                 .reinterpret(ints$LAYOUT().byteSize());
>>         }
>>         return Holder.SEGMENT;
>>     }
>> 
>>     /**
>>      * Getter for variable:
>>      * {@snippet lang=c :
>>      * int ints[2][3][4]
>>      * }
>>      */
>>     public static MemorySegment ints() {
>>         return ints$SEGMENT();
>>     }
>> 
>>     /**
>>      * Setter for variable:
>>      * {@snippet lang=c :
>>      * int ints[2][3][4]
>>      * }
>>      */
>>     public static void ints(MemorySegment varValue) {
>>         MemorySegment.copy(varValue, 0L, ints$SEGMENT(), 0L, ints$LAYOUT().byteSize());
>>     }
>> 
>>     private static VarHandle ints$ELEM_HANDLE() {
>>         class Holder {
>>             static final VarHandle HANDLE = ints$LAYOUT().varHandle(sequenceElement(), sequenceElement(), sequenceElement());
>>         }
>>         return Holder.HANDLE;
>>     }
>> 
>>     /**
>>      * Indexed getter for variable:
>>      * {@snippet lang=c :
>>      * int ints[2][3][4]
>>      * }
>>      */
>>     public static int ints(long index0, long index1, long index2) {
>>         return (int)ints$ELEM_HANDLE().get(ints$SEGMENT(), 0L, index0, index1, index2);
>>     }
>> 
>>     /**
>>      * Indexed setter for variable:
>>      * {@snippet lang=c :
>>      * int ints[2][3][4]
>>      * }
>>      */
>>     public static void ints(long index0, long index1, long index2, int varValue) {
>>         ints$ELEM_HANDLE().set(ints$SEGMENT(), 0L, index0, index1, index2, varValue);
>>     }
>> 
>> 
>> If the array element type is a struct, different code needs to be generated. Consider this global variable declaration:
>> 
>> 
>> struct Point { int x; int y; } points[2][3][4];
>> 
>> 
>> This generates the followi...
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add missing blank lines

src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java line 470:

> 468:                 }
> 469:                 """);
> 470:         if (!Utils.isStructOrUnion(varType) && !Utils.isArray(varType)) {

Why is this disabled for structs and arrays?

src/main/java/org/openjdk/jextract/impl/StructBuilder.java line 141:

> 139:         String offsetField = emitOffsetFieldDecl(varTree, javaName);
> 140:         if (Utils.isArray(varTree.type()) || Utils.isStructOrUnion(varTree.type())) {
> 141:             emitDimensionsFieldDecl(varTree, javaName);

Are we always emitting DIMS field, even for structs? Should this be moved into the `dims > 0` if block?

test/jtreg/generator/arrayAccess/array_access.h line 26:

> 24: #ifdef __cplusplus
> 25: extern "C" {
> 26: #endif // __cplusplus

This is only needed when we include this header in a C++ file. The ending `}` is also missing here. This could just be dropped I think.

test/jtreg/generator/arrayAccess/array_access.h line 59:

> 57: extern int ints1[2];
> 58: extern int ints2[2][3];
> 59: extern int ints3[2][3][4];

These need `EXPORT` too.

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

PR Review Comment: https://git.openjdk.org/jextract/pull/198#discussion_r1474411272
PR Review Comment: https://git.openjdk.org/jextract/pull/198#discussion_r1474414939
PR Review Comment: https://git.openjdk.org/jextract/pull/198#discussion_r1474427832
PR Review Comment: https://git.openjdk.org/jextract/pull/198#discussion_r1474429117