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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.08.2025 14:35, Frediano Ziglio wrote:
> On Fri, Aug 15, 2025 at 1:28 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 15.08.2025 12:53, Andrew Cooper wrote:
>>> 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.
>>
>> I don't care about the wording; what I do care about is to get the caveat
>> across. Maybe:
>>
>> '"auto" isn't used in its traditional sense, but rather with its C23 meaning.
>>  Such uses are intended to be limited to macro-local variables.'
> 
> Why limiting to macros?

Because, if I understood Andrew correctly, the specific goal is to help limit
what macros expand to.

Jan