Standardise and enforce code style

[martinling]

This PR applies a clang-format configuration that has been chosen to minimise change to the existing codebase, whilst still resulting in a consistent style across the project that can be applied and verified automatically. The configuration chosen does not conform to any specific human-readable style guide. It requires clang-format version 14 or higher.

First there are several commits which manually clean up code and comments which cannot be reformatted well automatically.

Next, // clang-format off is applied to areas with manual layout to be preserved.

Then the clang-format configuration is added, along with a new tools/reformat-source.sh script to apply it, and a formatting check is added to the CI.

Finally in the last commit, all code is reformatted to meet the new standard.

[mossmann]

Can you please change this from Jenkins to GitHub Actions? I've temporarily disabled Jenkins for this repo, and even when it can be returned to service I would prefer to use Jenkins only for things that require hardware.

[martinling]

Updated as follows:

• The CI check now uses this GitHub Action.
• Added a further commit which adds braces to any control statements that are missing them. This is not currently checked by clang-format, but we can add that check with the new InsertBraces option once version 15 is released.

We may still want to tweak the clang-format configuration further.

At the moment, the reformat touches about 15-20% of the code by line count. The biggest source of changes is the formatting of function types, with the rest mostly related to inconsistent whitespace. To some extent the function churn is inevitable since we were inconsistent before, but we should make sure we're happy with what we choose going forward.

What I have at the moment could be summarised as:

1. If the function prototype fits within 100 characters, put it on one line including the opening brace:

return_type function_on_one_line(type_one arg_one, type_two arg_two) {
        ...
}

2. Otherwise, put each argument on its own line, with the closing bracket and opening brace on a new line:

return_type function_on_multiple_lines(
        type_one long_argument_name_one,
        type_two long_argument_name_two,
        type_three long_argument_name_three
) {
        ...
}

The latter case doesn't actually match what we were doing before. Usually we put the closing bracket after the last argument, and the opening brace on its own line. This can be done by changing AlignAfterOpenBracket from BlockIndent to AlwaysBreak, and setting BraceWrapping: AfterFunc to true.

However, doing that creates two other problems:

1. Setting BraceWrapping: AfterFunc to true means that the opening brace will always be on its own line, even when the function prototype is short. This creates both a lot of churn and wasted space - we usually prefer a single line when possible, although notably not in libhackrf.

2. The AlignAfterOpenBracket setting is applied to function calls as well, not just declarations. And when we split calls over multiple lines, we usually prefer to put the closing bracket on a new line - we do this a lot in libusb_control_transfer calls in libhackrf, for instance, and I like it that way, because it adds a visual end to the indented arguments.

The other option would be AlignAfterOpenBracket: AlwaysBreak and BraceWrapping: AfterFunc: false, but I don't like that combination because it leaves no visual separation between the last argument and the first line of a function:

return_type function_on_multiple_lines(
        type_one long_argument_name_one,
        type_two long_argument_name_two,
        type_three long_argument_name_three) {
        first_line();
        second_line();
}

So for function protototypes it's basically a question of choosing which available combination to go with, unless we choose another linter entirely (I tried uncrustify, but found it a lot harder to get it close to our style overall).

Besides that, I'm mostly happy with the output, although there are some line wrapping decisions I dislike. These could probably be improved a bit by adjusting the various penalty factors in the configuration.

[martinling]

Updated as follows:

• Now setting AlignAlignAfterOpenBracket to AlwaysBreak rather than BlockIndent. This means that the closing bracket for a function call or declaration will go on the same line as the last argument, rather than on its own line. Also stops us running into this bug.

• Now setting BraceWrapping: AfterFunc to true, meaning that the opening brace of a function is always on its own line.

As I mentioned earlier, this combination does result in some extra churn, but on balance I think it's the better option going forwards. It also brings us closer to Linux kernel style, which has always nominally been the standard even if if not actually kept to.

• Reduced maximum line width to 90 characters. This still avoids most of the ugliness that would result from trying to squeeze things into 80, but plays nicer with GitHub, which typically starts needing horizontal scrollbars on diff snippets in PR conversations at around 95.

• Modified the penalties for how to wrap lines, specifically by setting all of PenaltyBreakAssignment, PenaltyBreakBeforeFirstCallParameter and PenaltyBreakOpenParenthesis to zero. This means that breaking after the = of an assignment, before the first parameter to a function, or after an opening parenthesis, are all "free", and will therefore be chosen before any other way to break a line.

This results in what seems to me to be very sensible wrapping behaviour, even when things get pushed quite close to the column limit. For an example demonstrating this, see here.

• Added a script which I used to run clang-tidy on the code - this requires first building everything with an additional CMake option to export the compilation database, excluding files not relevant to each hardware configuration, and also specifying where to find required headers. Currently this is just used as a very overengineered way to add missing braces to control statements, but we may want to look at other use of clang-tidy too, and this provides a starting point for that.

[metayan]

Nicely done.

I tried adding Cpp11BracedListStyle: false to get some space in {} lists but it does a bit more than I was hoping for. A simple SpacesInBraces option matching the other SpacesIn... options would be nice. Some day. ;)

[martinling]

Rebased after the recent PR merges. Also added a few more manual commits before the big reformat, to improve some odd output noticed on the previous review pass.