ClangFormat the C code

[hugovk]

Run ClangFormat on all of the C code to once and for all get rid of the mixed 8 tab/spaces -> all 4 spaces, get all the braces on a consistent style, enclose single lines in braces, and generally make it all consistent.

Re: #1770 (comment)

Will prevent bugs like #4492.

Let's do this after the 2020-04-01 release (#4354), when the PR list will be at a local minimum and to avoid merge conflicts.

There's four pre-commit hooks available, let's pick a good one to add to .pre-commit-config.yaml so the CI will prevent regressions.

We'll need to pick a style or define our own. Is there a predefined one close to the current style, or that is commonly used for Python extensions? What do other Python projects use?

[hugovk]

I made 5 branches for each predefined ClangFormat style:

• master...hugovk:clang-format-chromium
• master...hugovk:clang-format-google
• master...hugovk:clang-format-llvm
• master...hugovk:clang-format-mozilla
• master...hugovk:clang-format-webkit

For each, I first generated the config file:

clang-format -style=chromium -dump-config > .clang-format

Then flipped the SortIncludes value from true to false, as it breaks the build otherwise. This can be looked into later.

Then changed any *IndentWidth value from 2 to 4 spaces.

And then finally ran the formatter on the .c and .h files:

clang-format -i `git ls-tree -r HEAD --name-only | grep "\.[ch]$"`

---

One thing to note, I wrote:

Will prevent bugs like #4492.

This isn't actually true, ClangFormat doesn't insert brackets.

---

Any preferences?

[nulano]

They are all much easier to read than the current (lack of) style! Specifically, I like how Chromium and Mozilla (unlike some others) break multiline parameters put a single parameter per line, similar to the Python style. One good place to compare this are lines 90 to 110 in _imagingft.c. The other styles just try to fit as many as they can on each line.

There are a few places where the formatter makes the code worse no matter the style, hopefully these can be manually overriden per line. A few examples:
master...hugovk:clang-format-chromium#diff-5c9181156ebf1940ced5d6efcff08ab3L182
master...hugovk:clang-format-chromium#diff-5c9181156ebf1940ced5d6efcff08ab3R65-R75

Most styles also reference specific macros or file regexes to have special formatting, these might be a good place to customize. For example this section from llvm:

IncludeCategories:
  - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
    Priority:        2
  - Regex:           '^(<|"(gtest|gmock|isl|json)/)'
    Priority:        3
  - Regex:           '.*'
    Priority:        1

Compared with Google:

IncludeCategories:
  - Regex:           '^<ext/.*\.h>'
    Priority:        2
  - Regex:           '^<.*\.h>'
    Priority:        1
  - Regex:           '^<.*'
    Priority:        2
  - Regex:           '.*'
    Priority:        3
IncludeIsMainRegex: '([-_](test|unittest))?$'

A nice option could be to, for example, put Python headers first, optional libraries second (grouped by library?), Pillow last. This could even fix the SortIncludes setting.

[nulano]

Is there a predefined one close to the current style, or that is commonly used for Python extensions? What do other Python projects use?

Not sure whether there is a predefined style for ClangFormat, but PEP 7 specifies what should be used for CPython, and NumPy has their own extension of it.

It's probably not the prettiest, but while typing out what I don't like about it, I have gotten used to it already, and actually really like it now!

I feel like these two points are the most important (i.e. most variable among different styles):

• Function definition style: function name in column 1, outermost curly braces in column 1, blank line after local variable declarations.

static int
extra_ivars(PyTypeObject *type, PyTypeObject *base)
{
    int t_size = PyType_BASICSIZE(type);
    int b_size = PyType_BASICSIZE(base);

    assert(t_size >= b_size); /* type smaller than base! */
    ...
    return 1;
}

• Breaking long lines: if you can, break after commas in the outermost argument list. Always indent continuation lines appropriately, e.g.:

PyErr_Format(PyExc_TypeError,
             "cannot create '%.100s' instances",
             type->tp_name);

Without going into too much detail, I feel like the Mozilla style might be the closest. One difference I can see is that Mozilla declares pointers as int* number instead of PEP 7's int *number and the lack of a newline before else (the one part of PEP 7 I'm still not sure about 😃).

[hugovk]

Thanks for checking!

Specifically, I like how Chromium and Mozilla (unlike some others) break multiline parameters put a single parameter per line, similar to the Python style.

Agreed, it makes diffs easier to read too.

One difference I can see is that Mozilla declares pointers as int* number instead of PEP 7's int *number

That can be flipped with:

-PointerAlignment: Left
+PointerAlignment: Right

(Tested: works.)

and the lack of a newline before else (the one part of PEP 7 I'm still not sure about 😃).

And:

-  BeforeElse:      false
+  BeforeElse:      true

Tested: didn't work, must be some other config change needed too. But I'm also a bit less keen on it.

https://clang.llvm.org/docs/ClangFormatStyleOptions.html

[hugovk]

Please see PR #4770 for a suggestion using a "clang-format style that approximates Python's PEP 7".