Re: Coding Style Review and Automation

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Feb 10, 2025 at 11:16:09PM +0200, Oleksandr Andrushchenko wrote:
> Hello, everybody!
> 
> What is the rationale behind introducing a tool to help with coding style
> verification? I will partially quote Viktor Mitin here [2]:
> 
> "The Xen Project has a coding standard in place, but like many projects, the
> standard is only enforced through peer review. Such mistakes slip through and
> code is imported from other projects which may not follow the same standard. 
> The
> goal would be to come up with a tool that can audit the code base as part of a
> CI loop for code style inconsistencies and potentially provide corrections. 
> This
> tool is to embed as a part of the continuous integration loop."
> 
> I would add that it would better reflect reality to say that Xen's coding 
> style
> is quite incomplete to become a standard and needs some improvement to achieve
> that.
> 
> Here, I would like to provide a bit of history and acknowledge those brave
> individuals who dared and tried to introduce to Xen coding style checking and
> formatting support with clang-format.
> 
> Year 2017, Ishani Chugh.
> ---------------------------------------------------------------------
> This journey began with a request from an Outreachy program member [1].
> Then came the first patches by Ishani Chugh [2]
> Status: *busted*.
> 
> Year 2019, Viktor Mitin
> ---------------------------------------------------------------------
> Then picked up by Viktor Mitin, EPAM's first attempt [3].
> Status: *busted*.
> 
> Year 2020, Anastasiia Lukianenko
> ---------------------------------------------------------------------
> Continued by Anastasiia Lukianenko, EPAM's second attempt [4], [5].
> Some contributions were made to LLVM to make clang-format a better fit for
> Xen [6].
> Xen-summit and presentation [7] and the summary document [8].
> Status: *busted*.
> 
> Year 2023, Luca Fancellu
> ---------------------------------------------------------------------
> Luca restarted it, first ARM attempt [9], [10], [11].
> Status: *busted*.
> 
> That's all for now, but it is still impressive as of 2025.
> 
> So, in my opinion, what were the main issues with all these attempts? There 
> are
> many different views, but I would emphasize the following:
> 
> 1) clang-format doesn't perfectly fit Xen's code style as some rules it 
> applies
> are not liked by the community or it applies rules that Xen's coding style
> doesn't define (those Luca described in his .clang-format for every clang
> option).
> 
> 2) clang-format doesn't work in a "one-option-at-a-time" mode [12]: clang
> maintainers strongly oppose requests to allow turning off all options except
> some. Believe it or not, other maintainers also have strong opinions on what 
> is
> right and what is not for their projects, and this is one area where they will
> not compromise.
> 
> 3) The size of the patch after applying clang-format is huge. Really. 
> Something
> like 9 MB. Even if everyone agrees that the approach is good and we can 
> proceed
> with it, it is highly unlikely anyone will be able to review it. Considering
> that new patches are being added to the upstream during such a review, it may
> also lead to new code style violations or require a new review of that huge
> patch.

I think this approach is difficult.  It would likely introduce a lot
of noise when using `git blame` (I know, it's just one extra jump,
but...), plus would likely break every patch that we currently have
in-flight.

> 4) Which clang-format version should we set as the one used by Xen, so it is
> easy for everyone to use it on their hosts?
> 
> 5) You name it. I think many people in the community can name their points for
> and against clang-format.

What are the parts of our coding style that clang-format cannot
correctly represent?  Could you make a list of what would need to
change in Xen coding style for it to match perfectly what clang-format
will check?

Ideally the first step would be to prepare a patch to adjust the
coding style so it's in line with what clang-format will do.

Is it possible for clang-format to be applied exclusively to newly
added chunks of code, while keeping the surroundings untouched?

Thanks, Roger.