riccibruno added a comment.
In D83901#2155127 <https://reviews.llvm.org/D83901#2155127>, @MyDeveloperDay
wrote:
> clang-format -n warnings before this change in clang/test/Analysis/*.cpp
> (clang-format -n *.cpp |& grep warning | wc -l)
>
> before = 6903 vs after=6595, if it helps I'd say this looks good. A very
> brief review of changes seem to be mainly whitespace/extra lines and
> indentation (44 in headers files vs 43)
> (Note: There are 11 warnings after the first iteration, clang-format
> sometimes has to be run twice)
>
> The real proof is how many of the unit tests can be clang-formatted and still
> pass, but I completely approve of this effort, if the test directories were
> formatted this would give us a massive test suite of lots of different
> examples of c++ code to validate any clang-format changes.
>
> If having this in the clang-format file means users can use "Format on Save"
> options in their editors and not break tests then I think this is a good
> thing. As this will drive good behavior in all the other files rather than
> having to turn "Format on Save" off because we cannot maintain running tests.
>
> My 2 cents worth.
Thank you for your comment.
FWIW I think that the current approach with the lint bot needs to be changed
for the following reasons:
1. `clang-format` lets me forget about formatting and for this I like it very
much. This only works because running `clang-format` is almost always safe to
run. ie: it doesn't change the meaning of the code. This breaks down with most
tests in `test/` because the meaning of many test *is* changed by
`clang-format`: ie: `clang-format` is very much *not* safe to run on a test.
2. Even after running `clang-format` on a test and fixing it manually, the bot
will still complain in some cases if the formatting was done with a different
version of `clang-format`. This has happened to me multiple times and for some
reason always in a test. The bot should not complain if the formatting is fine
in one of the previous n versions.
3. Tests are written in a different style optimised for conciseness. For
example `struct S { S() = default; ~S() = delete; }; // not at all unusual in a
test`. This could be accounted for in `.clang-format` but...
4. ...`clang-format-diff` does not respect (as far as I know?) the
`.clang-format` in a sub-folder.
5. The lint bot is very noisy and makes reviews much harder because the
formatting complaints are inline. Moving them out-of-line would be significant
improvement.
I must admit I am very much tempted to put a generic `// clang-format off` in
the tests I write.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83901/new/
https://reviews.llvm.org/D83901
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
