Re: Clang-format configuration discussion - pt 2

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.11.2023 15:47, Luca Fancellu wrote:
> Hi all,
> 
> Let’s continue the discussion about clang-format configuration, this is part 
> 2, previous discussions are:
> 
>  - https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00498.html
> 
> You can find the serie introducing clang-format here:
> https://patchwork.kernel.org/project/xen-devel/cover/20231031132304.2573924-1-luca.fancellu@xxxxxxx/
> and there is also a patch linked to my gitlab account where you can find the 
> output for the hypervisor code.
> 
> For a full list of configurables and to find the possible values for them, 
> please refer to this page:
> https://clang.llvm.org/docs/ClangFormatStyleOptions.html
> 
> --------------------------------------------------------------------------------------------------------------------------------------------------
> 
> Our coding style doesn’t mention anything about alignment, shall we add a new 
> section?
> I can send patches when we reach agreement on each of these rules.
> 
> 
> QualifierAlignment: Custom
> QualifierOrder: ['static', 'inline', 'const', 'volatile', 'type']
> 
> ---
> For “QualifierAlignment” I chose Custom in order to apply in “QualifierOrder” 
> an order for the
> qualifiers that match the current codebase, we could specify also “Leave” in 
> order to keep
> them as they are.

Where do attributes go in this sequence?

> Depending on how the discussion goes on this one, it could be an entry in our 
> coding style
> 
> --------------------------------------------------------------------------------------------------------------------------------------------------
> 
> AlignAfterOpenBracket: Align
> 
> ---
> This one is to align function parameters that overflows the line length, I 
> chose to align them
> to the open bracket to match the current codebase (hopefully)
> 
> e.g.:
> someLongFunction(argument1,
>                                 argument2);

The above matches neither of the two generally permitted styles:

    someLongFunction(argument1,
                     argument2);

    someLongFunction(
        argument1,
        argument2);

Then again from its name I would infer this isn't just about function
arguments?

> This one can be a candidate for an entry in our coding style
> 
> --------------------------------------------------------------------------------------------------------------------------------------------------
> 
> AlignArrayOfStructures: Left
> 
> ---
> “When using initialization for an array of structs aligns the fields into 
> columns."
> It’s important to say that even if we specify “None”, it is going to format 
> the data structure anyway,
> I choose left, but clearly I’m open to suggestions.

You don't say in which way it re-formats such constructs.

> I don’t know how to phrase this one in our coding style
> 
> --------------------------------------------------------------------------------------------------------------------------------------------------
> 
> AlignConsecutiveAssignments: None
> 
> ---
> This one is disabled because of feedbacks from Stefano and Alejandro about 
> some weird behaviour on our
> codebase.
> 
> This one could be phased along this line: “Consecutive assignments don't need 
> to be aligned.”, the issue is
> that in this way it seems that it’s optional, but clang-format is going to 
> remove the alignment anyway for
> assignment that are consecutive and aligned.

Like below if there's no way to say "leave alone", then I don't consider this
usable. Imo we want to permit people to align when they think it helps, but
we don't want to demand them doing so.

Jan

> --------------------------------------------------------------------------------------------------------------------------------------------------
> 
> AlignConsecutiveBitFields: None
> 
> ---
> Same thing as AlignConsecutiveAssignments, but for bitfields.
> 
> --------------------------------------------------------------------------------------------------------------------------------------------------
> 
> AlignConsecutiveDeclarations: None
> 
> ---
> This aligns declarations names, same considerations as 
> AlignConsecutiveAssignments.
> 
> --------------------------------------------------------------------------------------------------------------------------------------------------
> 
> Ok this is it for now, let me know your thoughts about them, ideally if I 
> don’t get any reply in two weeks (7th of December),
> I will consider that we have an agreement on these configuration.
> 
> Cheers,
> Luca