[PATCH v11 03/25] gas: implement parsing of object attributes v2

[Jan Beulich]

On 16.01.2026 19:26, Matthieu Longo wrote:
> On 08/01/2026 13:04, Jan Beulich wrote:
>> On 31.12.2025 00:05, Matthieu Longo wrote:
>>> +  else if ((low_ttype & UNSIGNED_INTEGER) && signedness_issue) {}
>>> +    /* Already handled by extract_integer_literal(), nothing to do.  */
>>
>> Having the (empty) braces on the same line as the if() may be okay when the
>> body is really empty. With the comment, however, please place the braces as
>> usual.
>>
> 
> Fixed in the next revision.
> 
> Just to make sure that this is what you want:
> 
>    else if ((low_ttype & UNSIGNED_INTEGER) && signedness_issue)
>      {
>        /* Already handled by extract_integer_literal(), nothing to do.  */
>      }
> 
> not:
> 
>    else if ((low_ttype & UNSIGNED_INTEGER) && signedness_issue)
>      /* Already handled by extract_integer_literal(), nothing to do.  */
>      {}

I'd consider the latter acceptable, but I do prefer the former.

>>> +/* Parse the arguments of [vendor]_attribute directive.  */
>>> +static arg_t *
>>> +vendor_attribute_parse_args (obj_attr_vendor_t vendor ATTRIBUTE_UNUSED,
>>> +			     const obj_attr_subsection_v2_t *subsec ATTRIBUTE_UNUSED,
>>> +			     unsigned int nargs, ...)
>>> +{
>>> +  bool parsing_err = false;
>>> +  va_list args;
>>> +  va_start (args, nargs);
>>> +
>>> +  arg_t *args_out = xcalloc (nargs, sizeof (arg_t));
>>> +
>>> +  for (unsigned int n = 0; n < nargs; ++n)
>>> +    {
>>> +      if (! trim_whitespace_before_param (n + 1))
>>> +	{
>>> +	  ignore_rest_of_line ();
>>> +	  goto bad;
>>> +	}
>>> +
>>> +      arg_t *arg_out = &args_out[n];
>>> +
>>> +      arg_token_t expected_ttype = va_arg (args, arg_token_t);
>>> +      arg_token_t high_ttype = (expected_ttype & HT_MASK);
>>> +      /* Make sure that we called the right parse_args().  */
>>> +      gas_assert (high_ttype == ATTRIBUTE_KEY
>>> +	       || high_ttype == ATTRIBUTE_VALUE);
>>> +
>>> +      if (high_ttype == ATTRIBUTE_VALUE)
>>> +	{
>>> +	  arg_token_t type_attr_value
>>> +#if (TC_OBJ_ATTR_v1 && TC_OBJ_ATTR_v2)
>>> +	    = (subsec != NULL)
>>> +	    ? obj_attr_v2_get_arg_type (subsec->encoding)
>>> +	    : obj_attr_v1_get_arg_type (stdoutput, vendor,
>>
>> These two lines want indenting by two more positions.
> 
> Fixed.
> 
>>> +					args_out[n-1].val.u64);
>>> +#elif (TC_OBJ_ATTR_v1)
>>> +	    = obj_attr_v1_get_arg_type (stdoutput, vendor,
>>> +					args_out[n-1].val.u64);
>>> +#else /* TC_OBJ_ATTR_v2 */
>>> +	    = obj_attr_v2_get_arg_type (subsec->encoding);
>>> +#endif
>>
>> The #ifdef-ary here isn't very nice. Couldn't you drop all of it here,
>> leaving just the #if part, and have obj_attr_v{1,2}_get_arg_type()
>> return suitable "nothing" values when TC_OBJ_ATTR_v{1,2} isn't set?
> 
> This "nothing" value does not exist currently in arg_token_t.
> It means that I need to add new code to handle it.
> I would prefer to keep this #ifdefery rather than adding this magic value.
> 
> Is it ok to keep it as it currently is ?

In the interest of getting things in for 2.46 I would have said yes, but:
As written, the variable type_attr_value exists in all cases. To play safe
towards future additions, I would have expected such a "nothing" to exist
anyway, just to be used when something went wrong internally. Since
"nothing" wouldn't be needed if internals are correct, the answer is still
"yes", yet it's a hesitant one.

Jan