[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 4 inline comments as done.
erichkeane added inline comments.


================
Comment at: llvm/docs/CodingStandards.rst:1573
 
+Don't Use Braces on Simple Single-Statement Bodies of if/else/loop Statements
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
----------------
lebedev.ri wrote:
> `case` too
Case statements are troublesome, right?  First, i don't really see that ever being an issue (do we have people unnecessarily doing braces around case statements?), and I consider the conditions on when to use braces in case statements to be different.



================
Comment at: llvm/docs/CodingStandards.rst:1580
+statement is accompanied by a comment that loses its meaning if hoisted above the if
+or loop statement, or where the single statement is complex enough that it stops being
+clear that it is a single line. Note that comments should only be hoisted for loops and
----------------
jdoerfert wrote:
> hubert.reinterpretcast wrote:
> > I'm happy with the implications of how this is phrased, but I am not sure it was intended. A statement that is not going to be a single line (a loop inside an else) qualifies for braces.
> > A statement that is not going to be a single line (a loop inside an else) qualifies for braces.
> 
> I would agree to that. 
Well, I said 'statement', not 'line' to attempt to avoid this :)  I'll try a re-word, but I'd love additional suggestions.


================
Comment at: llvm/docs/CodingStandards.rst:1592
+    handleVarDecl(D);
+  else {
+    // In this else case, it is necessary that we explain the situation with this
----------------
jdoerfert wrote:
> hubert.reinterpretcast wrote:
> > I believe this is an example of bad style. Applying the prose text to the example:
> > Adding braces in this example to the above bodies do not introduce "meaningless lines of code" as the lines already occur regardless. Adding braces may arguably improve readability.
> > 
> > Say, for the following, the lack of uniformity in the use of braces is a distraction:
> > ```
> > if (A)
> >   zip();
> > else if (B) {
> >   foo();
> >   bar();
> > } else
> >   hello;
> > ```
> I also think a "compound statement" that has braces at some point can/should have them everywhere.
I've removed the 'lines of code' from meaningless, but I'd love to hear your suggestion on how to word this rule.  I couldn't come up with a set of rules that would be consistent with our current enforcement, and be reasonable.

I considered some rule where 'once an if/else tree gets braces, everything below that point' would have braces, but I didn't have a good wording for it (and I intended to not be novel here compared to enforcement).  In your case, the 'else' would have it, and I'd like the 'if' to be optional.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80947/new/

https://reviews.llvm.org/D80947