Re: [Xen-devel] Xen Coding style and clang-format

[Viktor Mitin <viktor.mitin.19@xxxxxxxxx>]

On Wed, Jul 31, 2019 at 2:25 PM Julien Grall <julien.grall@xxxxxxx> wrote:
>
>
>
> On 31/07/2019 12:16, Viktor Mitin wrote:
> > On Mon, Jul 29, 2019 at 3:35 PM Julien Grall <julien.grall@xxxxxxx> wrote:
> >> On 7/29/19 1:21 PM, Viktor Mitin wrote:
> >>> On Mon, Jul 29, 2019 at 1:49 PM Julien Grall <julien.grall@xxxxxxx> wrote:
> >>>> On 7/29/19 10:13 AM, Viktor Mitin wrote:
> >>>>> On Fri, Jul 26, 2019 at 3:50 PM Julien Grall <julien.grall@xxxxxxx> 
> >>>>> wrote:
> >>>>>>
> >>>>>> *****
> >>>>>>
> >>>>>> -    /* See linux
> >>>>>> Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */
> >>>>>> +    /* See linux
> >>>>>> +     * 
> >>>>>> Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */
> >>>>>>
> >>>>>> Multi-lines comment on Xen are using
> >>>>>> /*
> >>>>>>      * Foo
> >>>>>>      * Bar
> >>>>>>      */
> >>>>>
> >>>>> See my comment about clang-format supports only comments indentation 
> >>>>> for now.
> >>>>
> >>>> I saw it and I will reply here for simplicity. Having a automatic
> >>>> checker that will do the wrong things is not ideal.
> >>>>
> >>>> Imagine we decide to use this checker as a part of the commit process.
> >>>> This means that the code will be modified to checker coding style and
> >>>> not Xen one.
> >>>
> >>> Well, you are right. Even more, unfortunately, there is no such coding
> >>> style as 'bsd' in clang-format.
> >>> So 'xen' clang-format style is based on the 'llvm' style.
> >>
> >> Do you have a pointer to the LLVM style?
> >
> > Sure, see the next links:
> > https://github.com/viktor-mitin/xen-clang-format-example/blob/master/.clang-format_llvm
> > https://github.com/viktor-mitin/xen-clang-format-example/blob/master/.clang-format_xen
>
> That's clang-format configuration not a write-up easily readable by human. It 
> is
> also does not say what will happen for the rest of the things not configured 
> (if
> there are any).

Here is Clang-Format Style Options documentation:
https://clang.llvm.org/docs/ClangFormatStyleOptions.html

And LLVM Coding Standards documetation:
 https://llvm.org/docs/CodingStandards.html#introduction

Unfortunately, it seems it does not answer "what will happen for the
rest of the things not configured (if there are any)"...


>
> >
> >>
> >> As I pointed out in a different thread, it woudl be easier to start from
> >> an existing coding style (LLVM, BSD...) and decide whether you want to
> >> fully adopt it or make changes.
> >>
> >> So someone needs to be pick one and look at the difference in style with
> >> Xen. It seems you already done that job as you tweak it for Xen. Do you
> >> have a write-up of the differences?
> >
> > Yes, it is done exactly this way you mentioned. New 'xen' format style
> > is based on 'llvm'.
>
> Can you give a link to this write-up in a human readable way?

The summary I wrote in the original mail in this thread describes what
was done based on 'llvm' coding style of clang-format.
I'm putting it here with update: added BreakStringLiterals thing to be fixed.

Summary of the changes:
- Added 3 new formatting styles to cover all the cases mentioned in
Xen coding style document: Xen, Libxl, Linux;
- Added list of the files and corresponding style name mappings;
- Added indentation according to Xen coding style;
- Added white space formatting according to Xen coding style;
- Added bracing support exception for do/while loops;

Added to clang-format, however, probably this logic should be moved to
python part (see known clang-format limitations above):
- Braces should be omitted for blocks with a single statement. Note:
these braces will be required by MISRA, for example, so it is probably
worth adding such a requirement to the coding style.
- Comments format requirements. Note: //-style comments are defined in
C99 as well, and not just in the case of C++. C99 standard is 20-years
old…

To be added:
- Emacs local variables. Open points: Why to keep emacs local
variables in Xen code? What about other editors' comments (vim)?
- Warning to stderr in the case when ‘unfixable’ line/s detected.

To be fixed:
- Max line length from 80 to 79 chars;
- Disable // comments;
- Set BreakStringLiterals to false (not explicitly covered by Xen
coding style document for now)

>
> >
> >>
> >>>>>> I am not sure why clang-format decided to format like that. Do you 
> >>>>>> know why?
> >>>>>
> >>>>> The reason is that there are two strings in one line. It would not
> >>>>> change it if it were
> >>>>> not "arm,psci-1.0""\0", but "arm,psci-1.0\0".
> >>>>
> >>>> I would like to see the exact part of the clang-format coding style
> >>>> documentation that mention this requirements... The more that in an
> >>>> example above (copied below for simplicity), there are two strings in on
> >>>> line.
> >>>
> >>> The closest found seems BinPackParameters BinPackArguments,
> >>> however, it is about function calls according to manual...
> >>
> >> Above, you mention the work is based on the LLVM coding style. Is there
> >> anything in that coding style about the string?
> >
> > Well, not much. See clang-format configurator mentioned above.
> > However, there is a useful clang BreakStringLiterals option.
> > It should be turned off to follow your suggestion not to break string
> > literal for grep use case.
>
> I am not speaking about clang-format itself but the LLVM coding style. I 
> assume
> there is a human readable coding style for LLVM, right? If so, is there any
> section in it about string?

See the link above. Unfortunately, it is about C++ and not about C.
Seems there is no pure C support in clang-format.

>
> >
> >>
> >>>
> >>>>
> >>>>    >> +    D11PRINT("Allocated %#" PRIpaddr "-%#" PRIpaddr
> >>>>
> >>>>
> >>>>>
> >>>>>>
> >>>>>> *****
> >>>>>>
> >>>>>> -    clock_valid = dt_property_read_u32(dev, "clock-frequency",
> >>>>>> -                                       &clock_frequency);
> >>>>>> +    clock_valid =
> >>>>>> +        dt_property_read_u32(dev, "clock-frequency", 
> >>>>>> &clock_frequency);
> >>>>>>
> >>>>>> I am not sure why clang-format decide to format like that. The current 
> >>>>>> version
> >>>>>> is definitely valid.
> >>>>>
> >>>>> The current version is not valid as it takes 81 chars, while 79 is
> >>>>> allowed according to coding style.
> >>>>
> >>>> Really? I looked at the code and this is 62 characters... It would be 81
> >>>> characters if "&clock_frequency);" were on the same line. But see how it
> >>>> is split in 2 lines.
> >>>
> >>> You are right, there are two lines. So it needs to find out how to
> >>> configure or implement such a feature to ignore such cases.
> >>
> >> What's the LLVM coding style policy about this?
> >
> > Well, LLVM formats it as shown above. All the other 'native'
> > clang-format styles behave similarly.
>
> This does not answer to my question. You pointed me how clang-format is
> configured, not how the behavior of clang format for this particular case and
> the developer documentation related to this.

See the link above, hopefully it answers your question.

Thanks

>
> Cheers,
>
> --
> Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel