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

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

Hi Julien,

On Mon, Jul 29, 2019 at 3:35 PM Julien Grall <julien.grall@xxxxxxx> wrote:
>
> Hi Viktor,
>
> 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:
> >>
> >>> It is unknown how difficult it would be to implement that with
> >>> clang-format, however, it can be analyzed.
> >>
> >> ...  but the goal of this discussion is to understand the limitations of
> >> Clang-format and whether a Coding Style change may be easier.
> >
> > It is not so easy to do, so it may take a time to investigate every the 
> > case.
>
> There must be a documentation for clang-format to explain the default
> coding style and way to tweak it, right? Could we get a pointer to it?

https://clang.llvm.org/docs/ClangFormat.html
Even more, there is 'clang-format configurator' which allows
experimenting with clang-format options online:
https://zed0.co.uk/clang-format-configurator/

> >>>>
> >>>> *****
> >>>>
> >>>> -    /* 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

>
> 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'.

>
> >>>> 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.

>
> >
> >>
> >>   >> +    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.

Thanks

>
> Cheers,
>
> --
> Julien Grall

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