RFR: 7903584: Revisit struct accessors

[Jorn Vernee]

On Mon, 15 Jan 2024 16:32:29 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Drop the `$get` and `$set` from the getter/setter names.
>> 
>> To ensure that we don't run into any clashes with the indexed accessors on structs (e.g. because a `long` field setter would clash with the indexed getter), we replace the indexed accessors with a 'asSlice' method which can be used to slice a given pointer using an index. This turns the following access pattern:
>> 
>>     MemorySegment arr = ...
>>     int x = Foo.x$get(arr, 10); // old 
>> 
>> Into this:
>> 
>>     MemorySegment arr = ...
>>     MemorySegment theFoo = Foo.asSlice(arr, 10);
>>     int x = Foo.x(theFoo); // new 
>> 
>> This then allows us to drop the indexed accessors altogether, thus avoiding a name clash.
>> 
>> ---
>> 
>> This patch also drops the getters (both fro globals and struct fields) which return instances of a functional interface. There is an issue that the new getter name will clash with the FI instance getter (which was already using the nice name), but when discussing this with Maurizio we realized that returning wrappers, like these FI instances, is not really in the spirit of the current 'all-static' approach that jextract takes. So, this patch drops the FI instance getters. For now, the samples will manually wrap the address in an FI instance, until we address: https://bugs.openjdk.org/browse/CODETOOLS-7903628
>> 
>> Note: I did not test the LibffmpegMain and PanamaTime changes.
>
> src/main/java/org/openjdk/jextract/impl/StructBuilder.java line 193:
> 
>> 191:         appendIndentedLines(STR."""
>> 192: 
>> 193:             public static MemorySegment asSlice(MemorySegment ptr, long index) {
> 
> this might clash, right (although unlikely)? Perhaps we can detect clashes locally, e.g. inside StructBuilder?

We emit this method after all field accessors, so potentially we could collect all the field names, and then optionally append a `$` to the front of this name in case there is a clash.

> test/jtreg/generator/test8245003/Test8245003.java line 82:
> 
>> 80:         assertEquals(seg.byteSize(), Foo.sizeof());
>> 81:         assertEquals(Foo.count(seg), 37);
>> 82:         var greeting = Foo.greeting$slice(seg);
> 
> should this be renamed too? E.g. if there's a struct field, just generate a segment getter with the same name of the filed?

I wasn't sure here, since the semantics are different. We originally decided to name this method `$slice` instead of `$get` after all.

I'm fine with going for the simpler name.

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

PR Review Comment: https://git.openjdk.org/jextract/pull/175#discussion_r1452604078
PR Review Comment: https://git.openjdk.org/jextract/pull/175#discussion_r1452605102