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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to