Han-Wen Nienhuys <hanw...@gmail.com> writes: > On Mon, Jan 27, 2020 at 11:49 AM David Kastrup <d...@gnu.org> wrote: >> > I want to propose to move to automated formatting for our C++ code. >> > >> > I put up a .clang-format code that mostly mimicks our style at >> > >> > https://codereview.appspot.com/561340043 >> > >> > I have a lot of good experience with automating code formatting.. It >> > removes drudgery for code authors, obviates discussions over style in >> > code review, and generally elevates the level of discourse in our >> > reviews. >> > >> > What do you all think? >> >> scripts/auxiliar/fixcc.py and astyle 2.04 is what we standardized on. > > Yes, I am proposing that we change and standardize on clang-format. > >> > The current config modifies about 11k lines, mostly because of >> > different line breaks (necessary to keep the 80 column limit.) >> >> Any particular reason to change the automated style to a different one? > > Several reasons > > * clang-format is a *complete* formatter. It reformats files to a > canonical formatting regardless of how badly you mangle the input.
Do we want to mangle the input badly? > For example, if you introduce a comment line of 200 chars. In > clang-format, this will be reformatted to the specified column limit. > Astyle/fixcc doesn't know what to do with it. Comments often contain ASCII-art and formatted tables. Formatting those is not helpful. > * clang-format is based on clang itself, and has an understanding of > the source code being formatted. This makes it better than fixcc which > uses regular expressions (sic) to make sense of C++. This is > especially important as we move towards newer dialects of C++ with > newer constructs (eg. lambda). C++11 support is already in the release notes for Astyle 2.03 in 2013. > * Because it is so robust, it is safe to run automatically on save, > and there exists VIM and Emacs support to do so I don't understand what you call "safe" and "robust" here. Do you suggest that using Astyle would change the meaning of C++ code? > * fixcc is 500 lines of python that we could just stop maintaining. Fixcc does not much more than drive Astyle, and Astyle is being maintained by someone else. Here is its dependency list for the sake of people using lily-dev or similar: Depends: libc6 (>= 2.14), libstdc++6 (>= 5.2) Here is the dependency list of clang-format: Depends: libc6 (>= 2.14), libclang-cpp9, libgcc1 (>= 1:3.0), libllvm9 (= 1:9-2), libstdc++6 (>= 5.2), python3 So one dependency is python3, another is libclang-cpp9, and then there are various other compiler parts (both GCC and LLVM). I have put up an issue showing the current job fixcc/astyle would do on our code base. Do you see significant problems showing up there? > * clang-format is fast. I can format all of the LilyPond C++ code in 4 > seconds. That makes it fast enough to stick into a pre-commit hook > (check changed files for formatting). time scripts/auxiliar/fixcc.py --sloppy $(git ls-files '*.cc' '*.hh') real 0m12.651s user 0m10.593s sys 0m2.057s and I'd be willing to bet that your laptop runs faster than mine with its Intel(R) Core(TM) i7-2630QM CPU @ 2.00GHz . > * clang-format is configurable. I whipped a config that I think > matches what we have, but I might have missed some options. Ideally, > the changes would be small. The changes for AStyle are small because we already use AStyle. > * clang-format is well-established. Both the linux kernel and Git have > a .clang-format files in their trees. AStyle is well-established since we have been using it for years without a problem. If we get problems, that may warrant reevaluation. Can you point out problems in the sample I posted as Current branch: issue5703 Tracker issue: 5703 (https://sourceforge.net/p/testlilyissues/issues/5703/) Rietveld issue: 549480043 (https://codereview.appspot.com/549480043) Issue description: Run scripts/auxiliar/fixcc.py This is just intended to showcase the current fixes fixcc would cause. ? > Finally, commenting on Werner: there are two main positives for > automated formatting: I am not going to argue here since that is an old discussion with a resolution way in the past under Graham's tenure, and we opted for it. I forgot about the proper upkeep, and your reminder is very much appreciated. But we do have a working solution right now, even if it is my fault not to have minded it properly, and changing to one with vastly more dependencies and having to redefine formatting from scratch is something that I cannot, in the current situation, see as a winning proposition. In particular, pinning the operation to a particular version of Clang and making it automatic in an editor would make it a considerable headache for people to _not_ accidentally stomp back and forth over the formatting of files when their Clang version does not match that of other people. > 2. When code has standard formatting, it is much easier to build > automation for code changes, or do global search/replaces over the > code base. (This is probably not a big factor for LilyPond, but it's > what makes large scale automated refactoring feasible at places like > Google.) Actually, indentation changes are a nightmare for version control. That's why we decided already in the past to do them in batches. >> Clang is a pretty big dependency for developers. > > You'd think that, but it's not that bad: > > $ rpm -qi clang| grep Size > Size : 1695283 And how many dependencies are there in it? dak@lola:/usr/local/tmp/lilypond$ apt-cache depends clang-9 clang-9 Depends: libc6 Depends: libclang-cpp9 Depends: libgcc1 Depends: libllvm9 Depends: libstdc++6 Depends: libstdc++-8-dev Depends: libgcc-8-dev Depends: libobjc-8-dev Depends: libclang-common-9-dev Depends: libclang1-9 Depends: libc6-dev Depends: binutils Recommends: llvm-9-dev Recommends: python3 Recommends: libomp-9-dev Suggests: clang-9-doc dak@lola:/usr/local/tmp/lilypond$ apt-cache depends astyle astyle Depends: libc6 Depends: libstdc++6 Suggests: xdg-utils -- David Kastrup
