BUG: SystemABI C_LONG and C_LONGLONG are the same

[Ty Young]

On 5/18/20 5:15 AM, Maurizio Cimadamore wrote:
>
> On 16/05/2020 13:32, Ty Young wrote:
>>
>> On 5/15/20 6:41 AM, Maurizio Cimadamore wrote:
>>>
>>> On 15/05/2020 12:23, Ty Young wrote:
>>>>
>>>> On 5/15/20 5:09 AM, Maurizio Cimadamore wrote:
>>>>>
>>>>> On 15/05/2020 02:27, Ty Young wrote:
>>>>>> The issue with using Constable is that often times the 
>>>>>> information is the same, so I'm going to end up with not only 
>>>>>> more garbage but a really huge struct layout for every struct.
>>>>> Huge in what dimension? Source code? Memory footprint? ... ?
>>>>
>>>>
>>>> GC pressure, since ValueLayout instances are immutable. While it 
>>>> won't matter here specifically since it'll be GC'd once and done, 
>>>> I'm worried about cases where a ValueLayout(or any other layout 
>>>> type) needs to be put together on-the-fly very frequently.
>>>
>>> Then see the second suggestion I made about declaring your own set 
>>> of (static final) constants with CrossPoint-related info attached 
>>> once and for all.
>>>
>>>
>>
>> Yes, I know, but it's *ALWAYS* valid to append another attribute to 
>> any layout as part of its API, regardless of where or how it's being 
>> allocated(public static final constant, a for/while loop, etc). 
>> Immutable data structures are great and all but they aren't without 
>> their own issues.
>>
>>
>> Anyway, since this is the direction FMA is going(where was the prior 
>> discussion on this?), I guess I'll have to.
>>
>>
>> Some things I noticed:
>>
>>
>> A. It is not possible to add more attributes or a name without 
>> invalidating equality. In code:
>>
>>
>> System.out.println(NativeTypes.CHAR.equals(NativeTypes.CHAR.withName("test"))); 
>>
>>
>>
>> prints false, as expected but not wanted. I only thought of this when 
>> viewing struct layouts generated from jextract, which I didn't remove 
>> the "withName" from.
> I think what is missing from the API is a method to `drop` the 
> annotations - after which you can compare bare layouts. But, as it 
> stands, I stand by equals() also taking attributes into account.


In order to drop attributes you need to know which attributes to drop 
though, right? If someone is marking a layout with their own attributes 
willy nilly this may be impossible unless they provide the key strings. 
I also think it's a bit dangerous simply because they could mark a 
layout with the attributes but have the values be invalid. I can already 
see someone using a `long` layout that has an attribute "class" that 
points to byte.class.


The only way I can think of solving this is by introducing 
"ContableGroup". You could then have a method that strips a layout of 
all attributes but those within the supplied group. Doing this also has 
the added benefit that a signature could be added to validate who the 
attributes belong to.


>>
>>
>> B. Attribute values can be overwritten. This can maybe potentially 
>> result in a form of code manipulation.
> This should be fixed.


After thinking about it, instead of a blanket ban on overwrites, maybe 
introducing a method that explicitly allows overwriting of values and 
one that does not would be better? There may be uses cases where 
overwriting values is needed, although I can't think of any right now.



>>
>>
>> C. It isn't documented whether the attribute string is case 
>> sensitive(it is).
> This also should be clarified.
>>
>>
>> D. if you can just append whatever you want, is having dedicated 
>> methods really needed for a name and everything else?
>
> I think name is common enough to deserve a dedicated overload - it is 
> also treated very specially by the layout API - so many developers 
> could just rely on name() and forget about everything else. I don't 
> think there's other cases? (We used to have an 'ABItype()` but that 
> has been removed, for a similar reason as the one you suggest).


I would say one for the equivalent Java class might be useful, but with 
long(and other types, probably) it's kinda subjective on how one might 
represent it. Someone might argue that a long layout should always map 
to long.class, while others may have it point to int.class on Windows or 
long.class on Linux.


>
> Maurizio