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

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

Hi All,

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. For clarity, let’s
call such a tool as ‘Xen checkpatch tool’.

References for those who are interested in the background are in [5].

The idea of the new tool is to use the clang-format approach as a base
for Xen ‘checkpatch’ process. The new tool consists of modified
clang-format binary and modified clang-format-diff.py python script to
automate Xen patches format checking and reformatting. The tool can be
used as a pre-commit hook to check and format every patch
automatically. See the tool code under [1].

Known limitations:

Xen coding style
Xen coding style is defined in two 'CODING_STYLE' documents in Xen
code: Xen common coding style and Xen libxl coding style. The
documents describe some of the coding style rules. However, there is
no information about ‘base’ coding style used (i.e. K&R, Linux, LLVM,
Google, Chromium, Mozilla, WebKit…). For this reason, it is unclear
how to deal with some of the coding style rules not described in the
Xen coding style documents. See examples of the tool output under [2].

Clang-format
Generally, the design of clang-format is to only make formatting
changes, not adding or removing tokens (there are some exceptions to
this, like wrapping string literals). It means that clang-format can't
add or remove braces or change the style of the comments from C89 to
C++. Tool clang-tidy can make syntactic changes to the code. However,
unfortunately, clang-tidy is a heavyweight tool as it needs the
compile options to parse the file (See [3] and [4])

This can be clang generic limitation, e.g. we might want to add a
possibility to clang to alter the code, e.g. adding braces,
characters, etc". The concern here is that it seems it is against main
clang-format design principles, so those changes will not be
integrated into clang-format mainstream. It should be checked with
clang-format community first.

As an option, to overcome the limitations of clang-format tool in the
case of Xen coding style, it is possible to move some Xen code format
logic to the modified clang-format-diff.py tool.

Summary
To sum up, it is possible to automate Xen patches format checking and
corrections with some known clang-format limitations. Ideally, it
would be good to slowly migrate the entire code-base to be conforming,
thus eliminating the need for discussing and enforcing style issues
manually on the mailing list. The ‘Xen checkpatch tool’ provides the
closest approximation of the established Xen style (including styles
not formally spelled out by CODING_STYLE, but commonly requested).
The tool can be used as-is at the moment and improved later in case of
necessity.

The tool allows achieving automation of Xen patches format checking
and corrections with some known clang-format limitations (see below).
All the xen/*.c files have been tested with it.
See the results of the tool output under [2].

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;


The links:
[1] https://github.com/xen-troops/Xen-Clang-format/blob/devel/clang-format.patch
[2] 
https://raw.githubusercontent.com/viktor-mitin/xen-clang-format-example/master/0001-clang-format-checkpatch-output-example.patch
[3] https://developer.blender.org/T53211
[4] 
http://clang-developers.42468.n3.nabble.com/clang-format-add-around-statement-after-if-while-for-td4049620.html

[5]
Project status:
https://docs.google.com/document/d/10NJn-QvO1TvyJJJGE2PD6FtElYCT3neBAffIqeWHdiE/edithttps://docs.google.com/document/d/1tCcwZ9K38ToLGTPHZkfs2sS4s4YulrGoH8LIHwBMbg4/edit

Mailing list discussions:
https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg02848.html
https://lists.xenproject.org/archives/html/xen-devel/2017-04/msg00131.html
https://lists.xenproject.org/archives/html/xen-devel/2017-04/msg01739.html

Original implementation on GitHub:
https://github.com/sam5125/Xen-Clang-format


Thanks

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