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

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 31 06:40:14 PDT 2022


aaron.ballman 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
----------------
erichkeane wrote:
> mehdi_amini wrote:
> > 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.
> 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.
I'd like to make sure we're all reading this the same way, because I don't see this as being restrictive so much as formalizing what I think many reviewers have been asking for anyway. Here's some examples:
```
if (foo)  // Old rule: didn't ask for braces, new rule: asks for braces
  bar(1, 2,
        3, 4);
```
```
if (foo)  // Old rule: didn't ask for braces, new rule: asks for braces
  bar(1, 2,
        3, 4);
else  // Old rule: didn't ask for braces, new rule: doesn't ask for braces
  baz(1, 2);
```
```
if (foo) // Old rule: didn't ask for braces, new rule: doesn't ask for braces
  baz(1, 2);
else  // Old rule: didn't ask for braces, new rule: asks for braces
  bar(1, 2,
        3, 4);
```
```
if (foo) // Old rule: didn't ask for braces, new rule: doesn't ask for braces
  if (bar) // Old rule: didn't ask for braces, new rule: doesn't ask for braces
    baz(1, 2);
```
```
if (foo) // Old rule: didn't ask for braces, new rule: asks for braces
  if (bar(1, 2,
            3, 4)) // Old rule: didn't ask for braces, new rule: doesn't ask for braces
    baz(1, 2);
```
```
if (foo) // Old rule: didn't ask for braces, new rule: asks for braces
  for (int i = 0; i < 10; ++i) // Old rule: didn't ask for braces, new rule: asks for braces
    baz(i);
```
Am I interpreting this correctly?

(FWIW, I personally think it's better for us to err on recommending braces when they're unnecessary in these more complicated situations because that's the safer, more explicit style.)


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