[PATCH v1 03/14] gas: implement parsing of object attributes v2

[Matthieu Longo]

On 2025-04-25 09:28, Jan Beulich wrote:
> On 24.04.2025 18:27, Matthieu Longo wrote:
>> On 2025-03-28 14:24, Jan Beulich wrote:
>>> On 21.03.2025 18:14, Matthieu Longo wrote:
>>>> --- a/gas/config/obj-elf.c
>>>> +++ b/gas/config/obj-elf.c
>>>> @@ -41,6 +41,10 @@
>>>>    #include "elf/alpha.h"
>>>>    #endif
>>>>    
>>>> +#ifdef TC_AARCH64
>>>> +#include "elf/aarch64.h"
>>>> +#endif
>>>
>>> Why is this needed? It's imo bat practice to include arch-specific files here,
>>> even if ...
>>>
>>
>> Removed in the next revision.
>>
>>>>    #ifdef TC_MIPS
>>>>    #include "elf/mips.h"
>>>>    #endif
>>>
>>> ... context on both sides demonstrates that it was done before.
>>>
>>> For most of the new code following in this file, I'm again not overly happy to
>>> see so much dead code added for non-Arm64. Can't interested tc-*.h files have a
>>> #define added controlling whether this code is actually to be compiled?
>>>
>>
>> I refactored the code in the next revision, and shared the parsing code
>> between OAv1 and OAv2. So TC_* defines should not be necessary anymore.
>>
>> There is actually a lot of code added here for object attributes (both
>> v1 and v2). I am wondering if we could not move it to another file:
>> obj-attr.c
>> What do you think ?
> 
> Generally I like splitting things up into smaller files. Splitting up here
> may additionally make it easier to omit the code altogether for targets not
> using it. So - yes, please, if it's reasonably possible to arrange for.
> 

Split done in the next revision.

>>>> +  char* val = xmemdup0 (s, i);
>>>> +  arg_out->optional_set = true;
>>>> +  arg_out->type = STRING_VALUE;
>>>> +  arg_out->value.string = val;
>>>
>>> STRING_VALUE further up was used for string literals. How come it identifies
>>> what looks to be an identifier here? If it's an identifier, why would you not
>>> use one of the existing primitives to parse that, e.g. get_symbol_name()?
>>
>> get_symbol_name seems to do too much for what I needed, specifically the
>> handling of double quotes.
> 
> You could reject the double-quotes case after the function returned? Else
> there may be other suitable parsing functions already available? If
> absolutely nothing suitable is there, perhaps something wants adding, to
> avoid too much open-coding here?

I tried to use get_symbol_name, and it treats '-' as a delimiter, which 
is not correct for the name of subsections.

> 
> Jan

Matthieu