[llvm-dev] Codifying our Brace rules-

[Hal Finkel via llvm-dev]

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 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