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