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

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 31 06:35:14 PDT 2022


mehdi_amini added inline comments.


================
Comment at: llvm/docs/CodingStandards.rst:1596-1597
 
-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
+the following statement began. An ``if``/``else`` chain or a loop is considered
----------------
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.
Subjective isn't always bad :)
The coding standard is designed to leave some room indeed when we can't specify a rule that is always clear cut.


================
Comment at: llvm/docs/CodingStandards.rst:1633
-      if (shouldProcessAttr(A))
-        handleAttr(A);
   }
----------------
owenpan wrote:
> mehdi_amini wrote:
> > This example shows how this revision goes beyond do/while.
> This example was very similar to the one starting at line 1662. After taking out one level of nesting here, it won't seem redundant to the "Use braces on the outer block because there are more than two levels of nesting" example below.
I rather keep this example though if we don’t have a good reason to remove it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126512



More information about the llvm-commits mailing list