RFR: 7903606: Move layout and function descriptor generation closer to code builders

[Maurizio Cimadamore]

On Mon, 11 Dec 2023 17:01:23 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 167:
> 
>> 165:         appendIndentedLines(STR."""
>> 166:             public static \{type.getSimpleName()} \{javaName}$get(MemorySegment \{seg}) {
>> 167:                 return \{seg}.get(\{layoutString(1, layout)}, \{offset});
> 
> So, It seems that using the layout string directly here is fine since we only reference primitives?
> 
> Also, this (and the `set` method) don't need indentation. It results in this:
> 
> 
>     public static int x$get(MemorySegment seg) {
>         return seg.get(    JAVA_INT, 8);
>     }

what do you mean "using layout string directly" ? We have no other way to emit the layout string.

> src/main/java/org/openjdk/jextract/impl/Utils.java line 162:
> 
>> 160:             case Type.Declared declared when declared.tree().kind() == Declaration.Scoped.Kind.ENUM ->
>> 161:                 isPrimitive(((Declaration.Constant)declared.tree().members().get(0)).type());
>> 162:             case Type.Delegated delegated -> isPrimitive(delegated.type());
> 
> Not sure this is correct for pointers?

yeah, for pointer this should just return false

> src/main/java/org/openjdk/jextract/impl/Utils.java line 170:
> 
>> 168:     static Function getAsFunctionPointer(Type type) {
>> 169:         return switch (type) {
>> 170:             case Type.Delegated delegated -> getAsFunctionPointer(delegated.type());
> 
> Same here I think. e.g. not sure this is correct for pointers to function pointers (which are not function pointers themselves)

Not sure here - this function is meant to follow pointers until a function type is found. This method seems to do that?

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

PR Review Comment: https://git.openjdk.org/jextract/pull/156#discussion_r1423006694
PR Review Comment: https://git.openjdk.org/jextract/pull/156#discussion_r1423004446
PR Review Comment: https://git.openjdk.org/jextract/pull/156#discussion_r1423004870