Re: [PATCH v2] xen: Use auto as per C23

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/08/2025 11:25 am, Jan Beulich wrote:
> On 15.08.2025 11:51, Andrew Cooper wrote:
>> On 15/08/2025 10:36 am, Jan Beulich wrote:
>>> On 15.08.2025 10:33, Nicola Vetrini wrote:
>>>> On 2025-08-15 10:17, Andrew Cooper wrote:
>>>>> On 15/08/2025 8:20 am, Nicola Vetrini wrote:
>>>>>> On 2025-08-15 00:25, Andrew Cooper wrote:
>>>>>>> In macros it is common to declare local variables using typeof(param)
>>>>>>> in order
>>>>>>> to ensure that side effects are only evaluated once.  A consequence
>>>>>>> of this is
>>>>>>> double textural expansion of the parameter, which can get out of hand
>>>>>>> very
>>>>>>> quickly with nested macros.
>>>>>>>
>>>>>>> In C23, the auto keyword has been repurposed to perform type 
>>>>>>> inference.
>>>>>>>
>>>>>>> A GCC extension, __auto_type, is now avaialble in the new toolchain
>>>>>>> baseline
>>>>>>> and avoids the double textural expansion.
>>>>>>>
>>>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>>>> Reviewed-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
>>>>> Thankyou.
>>>>>
>>>>>>> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
>>>>>>> index 88bf26bc5109..38ef5d82ad95 100644
>>>>>>> --- a/xen/include/xen/compiler.h
>>>>>>> +++ b/xen/include/xen/compiler.h
>>>>>>> @@ -64,6 +64,20 @@
>>>>>>>  # define asm_inline asm
>>>>>>>  #endif
>>>>>>>
>>>>>>> +/*
>>>>>>> + * In C23, the auto keyword has been repurposed to perform type
>>>>>>> inference.
>>>>>>> + *
>>>>>>> + * This behaviour is available via the __auto_type extension in
>>>>>>> supported
>>>>>>> + * toolchains.
>>>>>>> + *
>>>>>>> + *
>>>>>>> https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Auto-Type.html
>>>>>>> + * https://clang.llvm.org/docs/LanguageExtensions.html#auto-type
>>>>>>> + */
>>>>>>> +#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 202311L
>>>>>>> +/* SAF-3-safe MISRA C Rule 20.4: Giving the keyword it's C23
>>>>>>> meaning. */
>>>>>>> +#define auto __auto_type
>>>>>>> +#endif
>>>>>>> +
>>>>>> A more detailed explanation should live in deviations.rst under this
>>>>>> bullet point
>>>>>>
>>>>>>    * - R20.4
>>>>>>      - The override of the keyword \"inline\" in xen/compiler.h is
>>>>>> present so
>>>>>>        that section contents checks pass when the compiler chooses not 
>>>>>> to
>>>>>>        inline a particular function.
>>>>>>      - Comment-based deviation.
>>>>>>
>>>>>> as described in the SAF entry:
>>>>>>
>>>>>>         {
>>>>>>             "id": "SAF-3-safe",
>>>>>>             "analyser": {
>>>>>>                 "eclair": "MC3A2.R20.4"
>>>>>>             },
>>>>>>             "name": "MC3A2.R20.4: allow the definition of a macro with
>>>>>> the same name as a keyword in some special cases",
>>>>>>             "text": "The definition of a macro with the same name as a
>>>>>> keyword can be useful in certain configurations to improve the
>>>>>> guarantees that can be provided by Xen. See docs/misra/deviations.rst
>>>>>> for a precise rationale for all such cases."
>>>>>>         },
>>>>> Ah right.  What about this:
>>>>>
>>>>> "Xen does not use the \"auto\" keyword as a storage qualifier.  The
>>>>> override of the keyword \"auto\" in xen/compiler.h is to give it it's
>>>>> C23 behaviour of type inference."
>>>>>
>>>>> ?
>>>> Seems good to me. Maybe this should be spelled out in ./CODING_STYLE as 
>>>> well, so that newcomers don't trip over this?
>>> I'm not sure newcomers would look there, but in the absence of any better
>>> place that's perhaps indeed where to mention this.
>> How about this:
>>
>> diff --git a/CODING_STYLE b/CODING_STYLE
>> index 7bf3848444ad..e33b9d1170cf 100644
>> --- a/CODING_STYLE
>> +++ b/CODING_STYLE
>> @@ -129,6 +129,10 @@ Fixed width types should only be used when a fixed 
>> width quantity is
>>  meant (which for example may be a value read from or to be written to a
>>  register).
>>  
>> +Macros which otherwise would use "typeof(arg) newarg =" to avoid double
>> +evaluation of side effects should use "auto newarg =" per it's C23 
>> behaviour,
>> +to also avoid double textural expansion.
>> +
>>  Especially with pointer types, whenever the pointed to object is not
>>  (supposed to be) modified, qualify the pointed to type with "const".
> That doesn't focus on the pitfall though, in that people shouldn't be using
> the "auto" keyword (except in said cases).

/sigh, this is why noone does patches to CODING_STYLE.

If you don't like the wording, propose some wording that you do like.

Or I will commit the patch without this hunk, because I'm not going to
get drawn into the cycle of blind guessing that every change to
CODING_STYLE seems to get caught in.

~Andrew