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