RFR: 7903606: Move layout and function descriptor generation closer to code builders
Maurizio Cimadamore
mcimadamore at openjdk.org
Mon Dec 11 19:01:46 UTC 2023
On Mon, 11 Dec 2023 17:04:41 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:
>> This PR moves calls to Type/Declaration.layoutFor/descriptorFor as close as possible to the leaf code builders. The reason for this change is to prepare the jextract code to generate layouts and descrptor strings directly from jextract types, so that we can retain as much information from the original clang cursor/types as possible when generating layouts.
>>
>> In order to get there, I had to add a lot of type predicates in the Utils class (perhaps some of these methods can be moved later directly in TypeImpl).
>>
>> And, perhaps the biggest change, is that previously we used layout presence/absence in `UnsupportedFilter` to make decision as to whether skip a certain declaration. For this reason, `UnsupportedFilter` (esp. its type visitor) has been enhanced to detect cases where layout generation would fail, but in a way that does not require us to generate layouts just to answer a question of "is this type supported?".
>
> 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).
> 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
-------------
PR Review Comment: https://git.openjdk.org/jextract/pull/156#discussion_r1422999302
PR Review Comment: https://git.openjdk.org/jextract/pull/156#discussion_r1423000443
More information about the jextract-dev
mailing list