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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
