RFR: 7903606: Move layout and function descriptor generation closer to code builders
Jorn Vernee
jvernee at openjdk.org
Mon Dec 11 19:08:55 UTC 2023
On Mon, 11 Dec 2023 18:57:49 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
>> src/main/java/org/openjdk/jextract/impl/StructBuilder.java line 188:
>>
>>> 186: appendIndentedLines(STR."""
>>> 187: public static MemorySegment \{javaName}$slice(MemorySegment \{seg}) {
>>> 188: return \{seg}.asSlice(\{offset}, \{size});
>>
>> I'm not so sure about embedding hard-coded offset and size literals into the generated code. Since, it is not clear what the number represents/where it comes from. This seems to clash with the goal of having the generated code be copy-pastable/understandable.
>>
>> I think if we go the route of dropping var handle-based accessors in favor of offset-based ones, we should instead generate `static final` offset and size constant fields, and then reference those here.
>
> I can generate a field, that's fine. But I do think that generating a var handle here is overkill. The main idea is that the struct has a layout accessor, which allows developers to get a var handle for a struct field _if needed_. In all existing usages of FFM/jextract the struct var handle accessor were never used, so I assumed that avoiding them would lead to better startup, while still leaving a powerful escape hatch (via the layout accessor).
Yes, I agree dropping the use of var handles here is preferable in general.
>> src/main/java/org/openjdk/jextract/impl/Utils.java line 119:
>>
>>> 117: default -> false;
>>> 118: };
>>> 119: }
>>
>> This method is only used by the other `isEnum`, and that one is unused (only called recursively). Coverage report also shows the code as dead.
>
> I have follow up code which depends on this - I can remove if preferred
Whichever you prefer then.
-------------
PR Review Comment: https://git.openjdk.org/jextract/pull/156#discussion_r1423004559
PR Review Comment: https://git.openjdk.org/jextract/pull/156#discussion_r1423004772
More information about the jextract-dev
mailing list