Panama source code fails to compile

[Maurizio Cimadamore]

On 04/01/2021 19:38, Ty Young wrote:
>>>
>>>
>>> - Padding layout should probably be a subclass of ValueLayout and 
>>> just override methods as needed. Having every memory layout have a 
>>> isPadding() method doesn't make much sense. The PaddingLayout class 
>>> can then be moved to an internal package as there is no way to 
>>> create an instance outside of ofPaddingBits as an API user. The same 
>>> could be done for AbstractLayout.
>> Are you talking about API or impl? Impl-wise, we could resort to 
>> overriding for isPadding, but if PaddingLayout is to be impl-private, 
>> then user has no way to do instanceof PaddingLayout, so you need a 
>> predicate?
>
>
> Bit of both. ofValueBits can be modified to return a ValueLayout 
> since, under this change, PaddingLayout extends ValueLayout and "is-a" 
> ValueLayout. isPadding() will then be made exclusive to ValueLayout 
> and would be how you determine if a ValueLayout is padding. If that 
> method exists, I don't see a reason why exposing PaddingLayout as part 
> of public API and letting a user do instanceof is needed as 
> isPadding() exists. isPadding() should maybe use getClass() instead of 
> instanceof, don't think it's a big deal but it's slightly faster IIRC. 
> There isn't any reason to expose PaddingLayout or AbstractLayout in 
> public API AFAIK.

Gotcha.

Yes, in principle padding can be a special case of value - except then 
you have endianness thrown in as well which isn't horrible, but doesn't 
make a lot of sense either.

My mental model is more:

LeafLayout
ValueLayout <: LeafLayout
PaddingLayout <: LeafLayout

which then led to the current compressed hierarchy.

>
>
>>>
>>>
>>> - GroupLayout should probably be split into StructLayout and 
>>> UnionLayout. This is the biggest change, but it simplifies the code, 
>>> provides better clarity on what's actually being represented, and 
>>> allows for future struct/union specific operations to be added 
>>> without adding a bunch of if(!isUnion) throw new 
>>> UnsupportedOperationException(); type code.
>>
>> Less sure about this.
>>
>> I think pattern matching will also help a great deal with this, at 
>> some point.
>
>
> My understanding is that you need a type to do pattern matching? The 
> Kind enum used internally for isStruct() and isUnion() is private.

For now you need a type, yes.

In the future, when pattern declaration will be available, we can add an 
extractor for each static factory there is in the API.

if (layout instanceof GroupLayout.ofStruct(layoutA, layoutB)) {
   ...
}

Something like that.

Maurizio

>
>
> If the issue is unifying the types then GroupLayout could maybe be 
> repurposed so that it simply exposes memberLayouts() and other shared 
> code?
>