Re: [RFC PATCH 5/5] xen: Add clang-format configuration

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 10 Aug 2023, Luca Fancellu wrote:
> > On 10 Aug 2023, at 09:08, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> > 
> > On 09.08.2023 18:18, Luca Fancellu wrote:
> >>> On 9 Aug 2023, at 16:48, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>> On 28.07.2023 10:11, Luca Fancellu wrote:
> >>>> --- /dev/null
> >>>> +++ b/xen/.clang-format
> >>>> @@ -0,0 +1,693 @@
> >>>> +# SPDX-License-Identifier: GPL-2.0-only
> >>>> +#
> >>>> +# clang-format configuration file. Intended for clang-format >= 15.
> >>>> +#
> >>>> +# For more information, see:
> >>>> +#
> >>>> +#   Documentation/process/clang-format.rst
> >>>> +#   https://clang.llvm.org/docs/ClangFormat.html
> >>>> +#   https://clang.llvm.org/docs/ClangFormatStyleOptions.html
> >>>> +#
> >>>> +---
> >>>> +
> >>>> +# [not specified]
> >>>> +# Align function parameter that goes into a new line, under the open 
> >>>> bracket
> >>>> +# (supported in clang-format 3.8)
> >>>> +AlignAfterOpenBracket: Align
> >>> 
> >>> I'm not convinced this rule (assuming I'm getting it right) is
> >>> suitable in all cases, especially for functions with long names or
> >>> very many parameters.
> >> 
> >> Not sure I understand, I think this is the current behaviour in the 
> >> codebase now.
> > 
> > My understanding is that the above aims at (only)
> > 
> > unsigned long function(unsigned long first_long_argument,
> >                       unsigned long second_long_argument);
> > 
> > when we also permit
> > 
> > unsigned long function(
> >    unsigned long first_long_argument,
> >    unsigned long second_long_argument);
> 
> Ohh ok now I see what you meant!
> 
> I did try this, ok sure it will format it as:
> 
> unsigned long function(unsigned long first_long_argument,
>                                      unsigned long second_long_argument);
> 
> Because it sees that it will fit the 80 chars.
> 
> At some point I think we need to overcome the uncertainty and chose a side, 
> only
> from there we can benefit from an automatic checker and we can get rid of 
> style issues (most of them).
> 
> Unfortunately most of the time we can’t say to the tool: we permit this style 
> but also this style, 
> only one can be allowed and just few discrepancy might be deviated if 
> properly justified (in-code comments).
> 
> I guess this is the only way to:
> 1) have a formalised coding style
> 2) have an automatic checker
> 3) unify the hypervisor codebase style, that will diverge (more than now) 
> very quickly with the introduction
>     of new arch and features

I think it is understood that we'll have to make some style choices that
today are left open and also that we might have to introduce some style
conversions. The question is exactly which choices and which
conversions. We need to come up with a list, hopefully a short one.