RFR: 7903485 Windows.h fails to extract on jextract/panama

Maurizio Cimadamore mcimadamore at openjdk.org
Mon Jun 12 11:34:06 UTC 2023


On Mon, 12 Jun 2023 09:55:34 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> src/main/java/org/openjdk/jextract/impl/TreeMaker.java line 217:
>> 
>>> 215:                 return null;
>>> 216:             }
>>> 217:             return Declaration.scoped(scopeKind, CursorPosition.of(c), c.spelling());
>> 
>> Could you explain this change? I guess we're assuming that there are no decls here since this is not a definition?
>
> To be 100% fair, I don't understand which situation is the `return` in the `else` supposed to cover. Even the old code seems off. E.g. `RecordLayoutComputer` seems to filter out cases where `c.getDefinition().isInvalid()`. But for everything else there should be a layout. But the layout was thrown away in the old impl as well. So, my goal here was to have a cleaner separation between "supported" cases and "unsupported" ones. Note that if we create a scoped decl w/o a layout (as we do in the old code), that thing ends up being skipped anyway by `OutputFactory`:
> 
> 
> if (d.layout().isEmpty() || structDefinitionSeen(d)) {
>             //skip decl
>             return null;
>         }
>  ```
>  
> Anyway, the main problem that was fixed here was to make sure that the _name_ of the type returned by `RecordLayoutComputer.compute` was reused, at least in the "supported" path. IMHO, we could just return `null` in the else, and we would probably not change semantics.

Did some more analysis, which refreshed my memory. When the cursor is not a definition, record layout computer will just return a fake declared type, backed by a padding layout. It will **not** look at any of the sub-cursors (as there's no definition for clang to see), so there will be no members attached to the scoped tree of the declared type. In other words, `decls` is guaranteed to be empty here.

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

PR Review Comment: https://git.openjdk.org/jextract/pull/122#discussion_r1226511952


More information about the jextract-dev mailing list