[foreign] RFR 8217781: Filter child cursors when doing recursive offset lookup

[Maurizio Cimadamore]

Looks good,
Some minor suggestions - these kind of issues arise because the code 
should never call cursor.children().

I suggest getting rid of isFlattenable, and have an accessor which does 
this:

Stream<Cursor> children(Cursor c) {
    return c.children()
                    .filter(RecordLayoutComputer::isFlattenable)
}

But then, as I was writing this, I realized that RecordLayoutComputer 
seems to have several helper methods which take a Cursor as input, when 
in reality the only input we ever pass is the record cursor itself 
(which is already stashed in a field) - so maybe we should get rid of 
these extra Cursor params and use the field?

Maurizio

On 25/01/2019 12:26, Jorn Vernee wrote:
> Hi,
>
> Based on JDK-8217664 [1], I have found the following bug with this 
> layout:
>
> ```
> struct Foo {
>     struct {
>         union {
>             int x;
>         } Bar;
>     };
> };
> ```
>
> To find the offset of the anonymous struct in Foo, we have to find the 
> offset of it's first field. But, the current code is looking for the 
> first child _cursor_ of the anonymous struct, which is the union 
> declaration, and not the field 'Bar'. This again tries to look up the 
> first field of 'Bar' which is 'x', so we end up looking up the field 
> 'x' in Foo, which throws an error.
>
> The fix is simple, we just have to filter for anonymous struct/union 
> and FieldDecl child cursors when doing a recursive offset lookup. I've 
> also added an exception message to the recursive lookup, which helped 
> debug this.
>
> Please review the following.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8217781
> Webrev: 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8217781/webrev.00/
>
> Thanks,
> Jorn
>
> [1] : https://bugs.openjdk.java.net/browse/JDK-8217664