[llvm-dev] Codifying our Brace rules-

[Adrian McCarthy via llvm-dev]

Personally, I favor "always use braces" because it helps readability for
me.  The compiler may be good at flagging misleading indentation, but my
visual processing system is terrible at it, especially since we use a
measly two spaces for indentation.  And we grant indentation exceptions
for--among other things--case labels in switches.

When some nested statements have braces and others don't, I can't rely on
visual pattern matching to grok the control flow at a glance (which is a
primary reason to standardize on brace and indentation styles in the first
place).  Instead I have to work out what goes with what and stuff those
answers into short-term memory as I work with the block.

I doubt we're going to move to wider indentation, and I don't expect I'll
convince anyone to indent case labels an extra level.  But the "always use
braces" rule is preferred by some here, is self-consistent, and is the only
distinction between LLVM's present (inconsistently applied) brace style and
the One True Brace Style[1].  It's mindless to apply and it helps
compensate for our stingy indentation.  If the vertical space used by close
braces seems too high a cost, I think that's an indication that there may
be too many levels of nesting or the function is too long or both.

[1]:  https://en.wikipedia.org/wiki/Indentation_style#Variant:_1TBS_(OTBS)

On Tue, Jun 23, 2020 at 7:40 AM Robinson, Paul via llvm-dev <
llvm-dev at lists.llvm.org> 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
>
> >
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200623/461d217b/attachment.html>