Replacing check-format.pl with clang-format

If anyone is curious, I've got this branch here:
https://github.com/nhorman/openssl/tree/sample-clang-format
With a ./clang-format file , a script ./utils/format-style.sh, and the updates to the tree for all the .[ch] files that results from it.

@Tim Hudson, thank you for the reminder

Back when we did the reformatting in early 2015, we went through a series of votes to figure out what approach to take - voting on each branch in turn working backwards. The vote text is below along with the result that gives the history.

• The 1.0.2 branch will be reformatted to comply with the new coding style

  PASS

• The 1.0.1 and 1.0.0 branches will be reformatted to comply with the new coding style

  PASS

• The 0.9.8 branch will be reformatted to comply with the new coding style

  PASS

• If the coding style is modified so that indents are 4 characters, these indents will use spaces (never combined into tabs)

  PASS

• When reformatting the codebase we will tag before and after automated reformatting, in all branches. Scripts to apply the GNU indent options will also be added to the repository. An announcement will be made a week in advance that this will happen and how it will happen.

  PASS

• We will perform the code reformat according to our previous decisions from 1st January. This must be completed prior to the next release. As soon as this vote passes (assuming it does) then we will make the announcement that the reformat will be taking place. Unless agreed otherwise between now and then we will use the following GNU indent options. If this produces output that conflicts with our approved style then we will work to address those conflicts later:

-nbad -bap -nbc -bbo -hnl -br -brs -c33 -cd33 -ncdb -ce -ci4 -cli0
-d0 -di1 -nfc1 -i4 -ip0 -l80 -lp -npcs -nprs -npsl -sai -saf -saw -ncs -nsc
-sob -nfca -cp33 -ss -ts0 -il1 -ppi1

FAIL

• We will give notice of our intention to reformat the code base immediately. One week later we will reformat as per our previous votes using Tim's openssl-format-source script. This will not be before the planned security release but will be immediately before the release of 1.0.2

  PASS

You can see the reformat on master both before and after the script was run via these links:

https://github.com/openssl/openssl/commits/master-pre-auto-reformat

https://github.com/openssl/openssl/commits/master-post-auto-reformat

Our rationale at the time (as I recollect) was to make it easy for others to see the impact of the script and to be able to perform checkouts of the state either side of the reformat if that was useful. We also wanted to not impact being able to easily backport fixes to active branches. Note we did not go back to earlier branches than 0.9.8.

I don't care what tool we use for this. I just want something that works almost all of the time. The current script doesn't.

@Neil Horman For historical reference, `indent` is what we used in the Big Source Code Reformat effort back in I can't remember when, but before the 1.1.0 release.

Also, to offer alternative tools to clang-format, I've experimented with the following:

1) indent

2) uncrustify

3) astyle

of the above, uncrustify has similar configurability to clang-format, and seems well maintained, but so far (anecdotally) clang-format seems to be the most widely used tool in this space.

That's true coding style covers not just style (format in terms of spacing etc.) but also coding practices (naming conventions, function exit points etc.) But still having a tool which will reliably handle format will allow us to focus on other details in code review process.

To the prior questions regarding our ability to match our current style document to features in clang-format. I've constructed the following table:

So, generally speaking, for those aspects of our style guide that might be checked with tools like check-format.pl, clang-format appears capable of providing the same checks and corrections. for the other items in our style guide (for instance Memory allocation rules), neither check-format.pl nor clang-format applies, as those elements are less about style than they are about (for lack of a better term) architecture.

But (to editorialize here somewhat), I don't think we should be thinking about a switch to clang-format in an effort to find a better tool to enforce our current style guide. I think instead, we should be looking at a new style format that is driven by what a tool (clang-format or otherwise) can provide for us in such a way that both checks and format corrections can be automated to provide consistency in our code base, consistency being (in my view) the most beneficial aspect of any style.

by way of argument, I'll assert that our existing style isn't really followed. Looking at the head of our master branch, we have about 903206 lines of code in .c and .h files. Running check-format.pl on each of those files, I find a total of 94410 errors reported, suggesting that approximately 10% of our C code violates our style guide as check-format.pl is currently written.

Why is that? anecdotally (fully admitting that I've not done any real counting of the causes), I think the reasons are:

1) Some errors are erroneous - i.e check-format.pl isn't coded to match some element of our current style, or has a bug in which it detects an error that really shouldn't be an error as we have our style documented

2) Sometimes we just decide that following our style isn't worthwhile in some cases (i.e. in a particular case the developer thought it was more readable in a way that didn't match style)

3) Some code pre-dates a subsequent update to the style guide, and was never brought up to date to match that style.

From that, I would conclude that our current situation is that we have a tool for our custom coding style that isn't consistently reflective of our style guide, that can't be used to do mass updates of our code base when our style changes (should we choose to do so), nor do we ourselves follow our coding style consistently (for various and sundry reason).

I think, rather than trying to form a style document that amounts to various guidance about how we like to see our code written, our style guide should instead focus on those elements that any formatting tool can't handle (malloc routines, naming, etc), and let the style be defined by the configuration of the tool we choose. That tool can be run automatically to detect errors on PR's (much like our current check-format CI job, but with a more robust tool than what we currently have), and more importantly be used by developers to automate the process of correcting those style changes.

The impacts are that (if we chose to do a mass code update to match style), there would be backporting impacts for a few releases, and it would of course be a new style to get used to (understanding that, even if we matched our tool config to our current style, there are aspects of our code that aren't covered by our style guide that other formatters would change).

The benefits however are that we would have a tool that is much more widely used and actively developed that our in house checking tools, relieving us of the burden to keep it up to date ourselves, making the process of style checking more robust, and automating the process of correcting style. The results I believe are going to be a far more consistent coding style, which, as I noted above, is really the biggest benefit, that in my view far outweighs any impact that this change might have.

@Frederik Wedel-Heinen Yes, that sounds to me like the way to go.

I’m all for the idea.

However, I also doubt that all of check-format.pl can be performed with clang-format.

So I think the question should be rephrased and the summary should explicitly state which checks should be done with clang-format instead of check-format.pl.

And then whatever checks remains has to be performed with check-format.pl.