[clang] Disable a few formatting options for test/

[riccibruno]

Summary

Hopefully this will make the bot a little less noisy. Rationale for each:

AlignTrailingComments: We don't want to force-align the various expected-error and friends.
CommentPragmas: Tell clang-format to leave the // CHECK: and the // expected- alone.
AlwaysBreakTemplateDeclarations: Templates in tests often have no break between the template-head and the declaration.

[MyDeveloperDay]

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.

[aaron.ballman]

I think this is a good improvement, but you should wait for a few days in case other reviewers have feedback.

[riccibruno]

In 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.

clang/test/.clang-format

 BasedOnStyle: LLVM
 ColumnLimit: 0
+AlignTrailingComments: false
+CommentPragmas: "(^ ?CHECK|^ ?expected-)"
+AlwaysBreakTemplateDeclarations: No