MyDeveloperDay added a comment. Fundamentally this looks ok to me, the biggest concern is fathoming out the change in TokenAnnotator.cpp to mean the same thing, but I think that is what the tests should be for I think.
One think I do is use the output of this https://clang.llvm.org/docs/ClangFormattedStatus.html it creates a file in `clang/docs/tools/clang-formatted-files.txt` these are all the files that when we last checked were 100% clang-formatted (all 7949) of them.. Now be careful because people don't always maintain that clean status (naughty them!), but I use to ensure I'm not breaking clang-format (for at least the default LLVM style) so build your own clang-format and then in the llvm-project directory I run clang-format -verbose -n -files clang/docs/tools/clang-formatted-files.txt This will check all the files (reasonably quickly YMMV) $ clang-format -verbose -n -files clang/docs/tools/clang-formatted-files.txt Clang-formating 7950 files Formatting [1/7949] clang/bindings/python/tests/cindex/INPUTS/header1.h Formatting [2/7949] clang/bindings/python/tests/cindex/INPUTS/header2.h Formatting [3/7949] clang/bindings/python/tests/cindex/INPUTS/header3.h Formatting [4/7949] clang/examples/Attribute/Attribute.cpp Formatting [5/7949] clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp Formatting [6/7949] clang/include/clang/Analysis/AnalysisDiagnostic.h Formatting [7/7949] clang/include/clang/Analysis/BodyFarm.h .... if your (or they more likely) have broken anything then you'll get a warning (in this case it was their fault) Formatting [134/7949] clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:19:16: warning: code should be clang-formatted [-Wclang-format-violations] namespace clang{ But this is a good way of giving clang-format a quick check to ensure its not broken anything (over large code base) - You will get failures (as this file is out of date) ================ Comment at: clang/docs/ClangFormatStyleOptions.rst:3708 + SpaceBeforeParens: Custom + SpaceBeforeParensFlags: + AfterFunctionDefinitionName: true ---------------- I'm not a massive fan of the use of 'Flags' in the config (I know we use it as the typename), naming things is hard! ================ Comment at: clang/include/clang/Format/Format.h:3416 + /// \version 14 + SpaceBeforeParensCustom SpaceBeforeParensFlags; + ---------------- I'm not a massive fan of the word `Flags` here and thoughts? ================ Comment at: clang/unittests/Format/FormatTest.cpp:14193 + verifyFormat("M (std::size_t R, std::size_t C) : C(C), data(R) {}", + SomeSpace2); } ---------------- IMHO I think we should see tests for the other combinations of custom (I know it might be repeated) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110833/new/ https://reviews.llvm.org/D110833 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
