[llvm-dev] Codifying our Brace rules-

[Fangrui Song via llvm-dev]

On 2020-06-23, Hal Finkel via llvm-dev wrote:
>
>On 6/23/20 9:39 AM, Robinson, Paul via llvm-dev wrote:
>>
>>>-----Original Message-----
>>>From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Jay Foad via
>>>llvm-dev
>>>Sent: Tuesday, June 23, 2020 4:47 AM
>>>To: Mehdi AMINI <joker.eph at gmail.com>
>>>Cc: llvm-dev at lists.llvm.org; Matt Arsenault <arsenm2 at gmail.com>
>>>Subject: Re: [llvm-dev] Codifying our Brace rules-
>>>
>>>On Tue, 23 Jun 2020 at 03:30, Mehdi AMINI via llvm-dev
>>><llvm-dev at lists.llvm.org> wrote:
>>>>On Mon, Jun 22, 2020 at 2:38 PM Steve Scalpone via llvm-dev <llvm-
>>>dev at lists.llvm.org> wrote:
>>>>>Me?  I would modify the first sentence from:
>>>>>
>>>>>>When writing the body of an if, else, or loop statement,
>>>>>>omit the braces to avoid unnecessary line noise. However,
>>>>>>braces should be used in cases where the omission of braces
>>>>>>harm the readability and maintainability of the code.
>>>>>To be:
>>>>>
>>>>>>Braces are optional around the body of an if, else, or loop
>>>statement,
>>>>>>except in cases where the omission of braces harm the readability and
>>>>>>maintainability of the code.
>>>>
>>>>The current wording is more clear as it expresses unambiguously the
>>>preferred way of formatting the code. I don't see a benefit to this change
>>>of phrasing (on the opposite, I prefer less ambiguous).
>>>
>>>I really don't like the current wording. It reads like "Do X. However,
>>>don't do X if ..." (where X is omit braces). I do not find this
>>>unambiguous! And it makes me unsure how to interpret "to avoid
>>>unnecessary line noise: did it really mean "omit the braces /if/ that
>>>avoids unnecessary line noise"?
>>Given the standard-mandated "line noise" required for lambdas, I
>>don't think avoiding line noise is much of an argument here.  If
>>you dislike line noise, C++ in general will annoy you.  As LLVM is
>>coded in C++, we have to live with it.
>>
>>If we're not going to mandate braces-always, we need guidance on
>>how to resolve a difference of aesthetic opinion between an author
>>and reviewer.  I've reviewed code where the author used a lot of
>>technically unnecessary parentheses in a long expression, where I
>>thought it reduced clarity and he thought it improved clarity; in
>>cases like this, the most insistent person wins, with a slight edge
>>to the reviewer who can refuse to LGTM the patch.  "Most insistent
>>person wins" isn't really a great basis for deciding on coding styles.
>>
>>A mix of aesthetic opinions in successive reviewers is what's most
>>likely to lead to a random mix of styles within a function/module,
>>and THAT will reduce clarity more than anything.
>>
>>The way around this is to suck up our aesthetic preferences and go
>>with hard-and-fast rules.  A few likely rule sets here:
>>
>>(1) always use braces, period.  This would be the easy to remember
>>and enforce, with some plusses as mentioned previously for IDEs and
>>not confusing clang-format into "fixing incorrect indentation" when
>>the problem is actually missing braces.
>>
>>(2) always use braces except for a one-line block.  This means
>>       for (blah)      // BAD
>>         if (yadayada)
>>           do_a_thing;
>>       for (blah) {    // GOOD
>>         if (yadayada)
>>           do_a_thing;
>>       }
>>
>>(3) always use braces except for a one-line statement, applied
>>recursively.  This means
>>       for (blah)      // GOOD
>>         for (whatever)
>>           if (yadayada)
>>             do_a_thing;
>>
>>Note that (2) and (3) would also need to lay down the law with
>>respect to if-else cases (each branch considered independently,
>>or all branches must be handled the same way).
>>
>>I acknowledge the irony that choosing between the above rules is
>>primarily an aesthetic choice when the goal was to eliminate
>>aesthetic choices; however, this is an aesthetic choice made by
>>the coding standard and *not* by the reviewer, which is my goal.
>>
>>--paulr

I am in the (3) camp. As Hubert mentioned, this can omit multiple levels of }:

```
       }
     }
   }
```

(1) "always use braces" is very annoying and greatly impairs my comprehension
of a block of code.

In Adrian's example, if there is a comment, I'd prefer braces:

   if (condition) {
     // there's a comment here
     Statement = Something;
   } else {
     ComputeThisOrThat();
   }

For Aaron's example, I'd prefer:

   if (foo)
     bar(); // comment

but I'm not so strongly opposed to 

   if (foo) {
     bar(); // comment
   }

>I don't have a strong opinion on exactly what rules we choose. I've 
>gotten used to the style of omitting braces for single-statement 
>bodies, and at least for me, this generally improves my comprehension 
>of the code because it allows me to see more of it at once. I agree 
>with Paul, however, that we should have rules (and, like all rules, 
>they'll appear suboptimal in some circumstances). That's better than 
>having the style determined by some fairly-arbitrary combined 
>aesthetic sense of each patch author and its set of reviewers.
>
> -Hal
>
>
>>
>>>Jay.
>>>_______________________________________________
>>>LLVM Developers mailing list
>>>llvm-dev at lists.llvm.org
>>>https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>_______________________________________________
>>LLVM Developers mailing list
>>llvm-dev at lists.llvm.org
>>https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>-- 
>Hal Finkel
>Lead, Compiler Technology and Programming Languages
>Leadership Computing Facility
>Argonne National Laboratory
>
>_______________________________________________
>LLVM Developers mailing list
>llvm-dev at lists.llvm.org
>https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev