Change to clang-format
It might be beneficial to switch to clang-format because 1) it has better modern C++ support and 2) it is designed fully automatic formatting which allows fully automatic refactoring.
The most recent attempt of doing the switch was in Jan 2017. The gerrit issues can be found at:
As a result multiple bugs were identified as affecting us in an important way (many newly filed by us):
This could be revisited when the important of those have been resolved.
#1 Updated by Roland Schulz 11 months ago
Besides random bugs, our version of uncrustify misformats braced initializer lists and ">>" in templates. It might help to upgrade to a newer version. But this won't address all issues. In particular I think we would benefit from having a maximum line length but uncrustify isn't able to split strings automatically. And most of our insanely long lines are because of long strings.
I personally thing that the advantages of clang-format outweigh the disadvantages by far. And I even think that's true with pretty much any clang-format configuration. It would be nice to know what people consider blocking issues in clang-format. If there are only a few relatively easy to fix blocking issues I would look into fixing those in clang-format.
#2 Updated by Roland Schulz 11 months ago
This was discussed at today's call.
An extra issue was raised: We often align with more spaces than necessary. clang-format doesn't support this and would likely be hard to upstream this functionality because it seems to be not inline with its design goals. This has impact on the size of the initial reformatting change. It also means that adding/removing lines can trigger white-space changes on neighboring lines more easily.
As possible solution too the large number of changes in the original reformat, only reformatting lines as they get changed, was mentioned. This is possible with clang-format-diff. This might be sightly inconsistency between new lines formatted with clang-format and old lines. It also might mean that alignment is not always consistent between modified and unmodified lines.
#4 Updated by Roland Schulz 11 months ago
We could patch clang-format. But I'm less exited about making changes which we don't try to upstream. Both because this requires everyone to install a modified version of clang-format (and that is harder to distribute and build than uncrustify) and because it requires continued effort to rebase those changes if we want to update clang-format semi regularly.
#5 Updated by Mark Abraham 11 months ago
I'm 100% in favour of vanilla clang-format (we considered it some years ago and concluded that we really wanted something coming in version 3.8, but that's fine now). Getting new devs on board easily is much more valuable than the cost to existing devs of bumping the indentation one time (if we have to). Rebasing over that patch is a simple matter of accepting the changed code, and then applying the formatter.
We should switch to clang-format as soon as we can.
There didn't seem to be strong objections to the switch and the formatting suggestion that Roland uploaded. IIRC I had the strongest objections, but I can live with the things I don't like so much, in particular spaces around * and /.
I just looked at the formatting again and there I commented on some minor inconsistencies. I also noticed several alternatives from pointer * alignment Roland mentioned. Is there a clear preference for one of these?
#8 Updated by Eric Irrgang 12 days ago
Some additional tasks:
- [ ] update admin/uncrustify.sh (or its dependents)
- [ ] update reformat_all.sh
- [ ] update docs/dev-manual/uncrustify.rst
- [ ] update docs/release-notes/2020/major/tools.rst
- [ ] update jenkins.rst (and presumably jenkins)
I won't bother attaching a complete
uncrustify in the source tree...