[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/include/clang/Format/Format.h:3416
+  /// \version 14
+  SpaceBeforeParensCustom SpaceBeforeParensFlags;
+
----------------
HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > I'm not a massive fan of the word `Flags` here and thoughts?
> Yeah, neither, but Options is taken.
> 
> But Flags indicate that these will always be booleans, and we know that may change in the future.
> 
> Is it possible the reuse SpaceBeforeParensOptions as struct and still parse the old enum? (Yes of course, but is it feasible in terms of needed work?)
but "Options" as in SpaceBeforeParensOptions  is the type not the name so we could use SpaceBeforeParensOptions 

personally I would change the typename of SpaceBeforeParensOptions  to avoid confusion but its not essential as it would be 

`SpaceBeforeParensCustom  SpaceBeforeParensOptions;`

for me I sometimes like using Struct anyway

`SpaceBeforeParensStruct SpaceBeforeParensOptions;`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110833/new/

https://reviews.llvm.org/D110833