[PATCH] D126512: [Docs] Clarify the guideline on omitting braces

[Erich Keane via Phabricator via llvm-commits]

erichkeane added a comment.

I'd prefer we limit the scope of this to JUST a 'do-while' loop.  I think we can clarify the "if an inner statement uses braces, the outer one should too", AND special-case the do-while loop as needing braces.

BUT THIS, for example:

  if (isa<VarDecl>(D))
     for (auto *A : D.attrs())
       if (shouldProcessAttr(A))
         handleAttr(A);

Which I think the 'single line' rule you post breaks.  Also, my interpretation of what you wrote ALSo breaks 1649 in the 'after'.

should remain the way.



================
Comment at: llvm/docs/CodingStandards.rst:1596
 
-This list is not exhaustive, for example, readability is also harmed if an
+Similarly, braces should be used when a single-statement body can't fit on a
+single line; otherwise, it would be difficult to see where the block containing
----------------
owenpan wrote:
> mehdi_amini wrote:
> > This is new and seems overly restrictive to me, why this new change?
> See the second paragraph of the patch summary above. The phrase "complex enough" is too subjective to be useful IMO, but I will withdraw this specific change if you insist.
I'm not really a fan of this one.  I DO really like the 'cascading single-liners', and want to keep those omitting braces. The example @mehdi_amini  pointed out is important to me to keep.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126512