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