[foreign] RFR: record padding is not emitted correctly

[Maurizio Cimadamore]

On 14/06/18 16:54, Henry Jen wrote:
> I believe in getRecordLayoutInternal, line 353, stream filter should use cx.isAnonymous rather than cx.isAnonymousStruct, as an anonymous union can be embedded in a struct.
That's deliberate - it was anonymous() before, but that was too wide of 
a net and also caught enums - anything that did not have a name, not 
just records. I looked at the clang API and it seems this method speaks 
about records (e.g. STRUCT | UNION). The test has an anon union, and it 
works fine here, so I think we're all right?

What I want is this:

https://clang.llvm.org/doxygen/group__CINDEX__TYPES.html#ga6e0d2674d126fd43816ce3a80b592373

What Cursor.isAnonymous gives me is this:

public boolean isAnonymous() {
         return isAnonymousStruct() || isAnonymousEnum();
     }

Maurizio

>
> Otherwise, seems good to me. I’ll do some testing later.
>
> Cheers,
> Henry
>
>> On Jun 14, 2018, at 7:37 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>
>> Hi,
>> this is an overhaul of how jextract adds padding to record layouts - the general strategy is this:
>>
>> 1) make sure that fields are emitted at the correct expected offset, if not , add prefix padding
>> 2) make sure that record size matches the expected one, if not add trailing padding
>>
>> Note that (1) should onlyy apply to structs, not unions, as for unions, you always restart from the base offset for each field; so, assumption is, base offset should already be in sync because of outer padding (if needed).
>>
>> To get there, the Utils::getRecordLayoutInternal now does a recursive walk, as anonymous nested records must join the dance too.
>>
>> I also did a followup fix on the patch that Sundar sent yesterday; in short, using Utils:getIdentifier doesn't always work. For typedefs it does the right thing, but for anonymous records it just gives a random blob of text mentioning the source location; I tweaked this so that if I see that the output contains '::', I generate a synthetic string "anon$xyz", where xyz is the offset in the source for that anon record.
>>
>>
>> I also noted that when structs contain incomplete arrays (e.g.flexible array members), clang fails to return correct offset for anything inside the struct, and returns -2 instead. Right now I'm accepting it (as did the old impl, and the one in the nicl branch), but I believe it would be better to punt with an exception (after all the generated layout will be all wrong) - but if I do so, the StructTest, which contains an incomplete array, will fail. Opinions?
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/padding_layout/
>>
>> Cheers
>> Maurizio
>>