[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

Marek Kurdej via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 28 04:23:15 PST 2021


curdeius added a comment.

Could you have a look at preceding reviews and see if there wasn't a similar patch before?
I think that this option is a bit too limited.
Only removing braces doesn't seem enough.
Also, one should probably be able to decide when to add/remove them by e.g. setting the number of lines in what's considered short blocks.



================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3398
 
+**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14`
+  Remove optional braces of control statements (``if``, ``else``, ``for``,
----------------
I don't think it's a good idea to make this option dependent on a particular style. Even if it works for LLVM style only for the moment.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3419
+   if (isa<VarDecl>(D)) {              vs.     if (isa<VarDecl>(D)) {
+     for (auto *A : D.attrs()) {                 for (auto *A : D.attrs())
+       if (shouldProcessAttr(A)) {                 if (shouldProcessAttr(A))
----------------
I'm not sure if the braces on the right should be removed in the for loop.
There should probably be an option to set the  minimum number of lines/statements inside a control statement to control adding/removing braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116316



More information about the cfe-commits mailing list