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

[Julien Grall <julien.grall@xxxxxxx>]

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

> > > > > > 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?
> > > >    >> +    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.
Cheers,

--
Julien Grall

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