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

[Jorn Vernee]

On Mon, 12 Jun 2023 11:29:29 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> 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.

I thought perhaps the `else` was covering opaque types. i.e. in a case where we just have `typedef struct Opaque* opaque_t`, there would still be a `Declaration.Scoped` in the tree without a layout.

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

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