[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add flags

[MyDeveloperDay via Phabricator via cfe-commits]

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