[llvm] c08ea07 - Add to the Coding Standard our that single-line bodies omit braces

[Erich Keane via llvm-commits]

Author: Erich Keane
Date: 2020-06-11T12:46:15-07:00
New Revision: c08ea0771682de45964dc10759d6a57ce65c482d

URL: https://github.com/llvm/llvm-project/commit/c08ea0771682de45964dc10759d6a57ce65c482d
DIFF: https://github.com/llvm/llvm-project/commit/c08ea0771682de45964dc10759d6a57ce65c482d.diff

LOG: Add to the Coding Standard our that single-line bodies omit braces

This is a rule that seems to have been enforced for the better part of
the decade, so we should document it for new contributors.

Differential Revision: https://reviews.llvm.org/D80947

Added: 
    

Modified: 
    llvm/docs/CodingStandards.rst

Removed: 
    


################################################################################
diff  --git a/llvm/docs/CodingStandards.rst b/llvm/docs/CodingStandards.rst
index 767636531215..adf6e9036941 100644
--- a/llvm/docs/CodingStandards.rst
+++ b/llvm/docs/CodingStandards.rst
@@ -669,15 +669,15 @@ copy.
 .. code-block:: c++
 
   // Typically there's no reason to copy.
-  for (const auto &Val : Container) { observe(Val); }
-  for (auto &Val : Container) { Val.change(); }
+  for (const auto &Val : Container) observe(Val);
+  for (auto &Val : Container) Val.change();
 
   // Remove the reference if you really want a new copy.
   for (auto Val : Container) { Val.change(); saveSomewhere(Val); }
 
   // Copy pointers, but make it clear that they're pointers.
-  for (const auto *Ptr : Container) { observe(*Ptr); }
-  for (auto *Ptr : Container) { Ptr->change(); }
+  for (const auto *Ptr : Container) observe(*Ptr);
+  for (auto *Ptr : Container) Ptr->change();
 
 Beware of non-determinism due to ordering of pointers
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -884,7 +884,7 @@ It is much preferred to format the code like this:
 .. code-block:: c++
 
   Value *doSomething(Instruction *I) {
-    // Terminators never need 'something' done to them because ... 
+    // Terminators never need 'something' done to them because ...
     if (I->isTerminator())
       return 0;
 
@@ -896,7 +896,7 @@ It is much preferred to format the code like this:
     // This is really just here for example.
     if (!doOtherThing(I))
       return 0;
-    
+
     ... some long code ....
   }
 
@@ -1000,7 +1000,7 @@ Or better yet (in this case) as:
       Type = Context.getsigjmp_bufType();
     else
       Type = Context.getjmp_bufType();
-    
+
     if (Type.isNull()) {
       Error = Signed ? ASTContext::GE_Missing_sigjmp_buf :
                        ASTContext::GE_Missing_jmp_buf;
@@ -1010,7 +1010,7 @@ Or better yet (in this case) as:
 
 The idea is to reduce indentation and the amount of code you have to keep track
 of when reading the code.
-              
+
 Turn Predicate Loops into Predicate Functions
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -1081,7 +1081,7 @@ In general, names should be in camel case (e.g. ``TextFileReader`` and
 * **Variable names** should be nouns (as they represent state).  The name should
   be camel case, and start with an upper case letter (e.g. ``Leader`` or
   ``Boats``).
-  
+
 * **Function names** should be verb phrases (as they represent actions), and
   command-like function should be imperative.  The name should be camel case,
   and start with a lower case letter (e.g. ``openFile()`` or ``isFoo()``).
@@ -1091,7 +1091,7 @@ In general, names should be in camel case (e.g. ``TextFileReader`` and
   discriminator for a union, or an indicator of a subclass.  When an enum is
   used for something like this, it should have a ``Kind`` suffix
   (e.g. ``ValueKind``).
-  
+
 * **Enumerators** (e.g. ``enum { Foo, Bar }``) and **public member variables**
   should start with an upper-case letter, just like types.  Unless the
   enumerators are defined in their own small namespace or inside a class,
@@ -1107,7 +1107,7 @@ In general, names should be in camel case (e.g. ``TextFileReader`` and
         MaxSize = 42,
         Density = 12
       };
-  
+
 As an exception, classes that mimic STL classes can have member names in STL's
 style of lower-case words separated by underscores (e.g. ``begin()``,
 ``push_back()``, and ``empty()``). Classes that provide multiple
@@ -1359,7 +1359,7 @@ prefer it.
 The use of ``#include <iostream>`` in library files is hereby **forbidden**,
 because many common implementations transparently inject a `static constructor`_
 into every translation unit that includes it.
-  
+
 Note that using the other stream headers (``<sstream>`` for example) is not
 problematic in this regard --- just ``<iostream>``. However, ``raw_ostream``
 provides various APIs that are better performing for almost every use than
@@ -1491,7 +1491,7 @@ being closed by a ``}``.  For example:
   public:
     explicit Grokable() { ... }
     virtual ~Grokable() = 0;
-  
+
     ...
 
   };
@@ -1540,8 +1540,8 @@ as possible, and only use them for class declarations.  For example:
   };
   } // end anonymous namespace
 
-  static void runHelper() { 
-    ... 
+  static void runHelper() {
+    ...
   }
 
   bool StringSort::operator<(const char *RHS) const {
@@ -1569,6 +1569,53 @@ you have no immediate way to tell if this function is local to the file.  In
 contrast, when the function is marked static, you don't need to cross-reference
 faraway places in the file to tell that the function is local.
 
+Don't Use Braces on Simple Single-Statement Bodies of if/else/loop Statements
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+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.
+
+Readability is harmed when a single statement is accompanied by a comment that loses
+its meaning if hoisted above the ``if`` or loop statement. Similarly, braces should
+be used when single-statement body is complex enough that it becomes 
diff icult to see
+where the block containing the following statement began. An ``if``/``else`` chain or
+a loop is considered a single statement for this rule, and this rule applies recursively.
+This list is not exhaustive, for example, readability is also harmed if an
+``if``/``else`` chain starts using braced bodies partway through and does not continue
+on with braced bodies.
+
+Maintainability is harmed if the body of an ``if`` ends with a (directly or indirectly)
+nested ``if`` statement with no ``else``. Braces on the outer ``if`` would help to avoid
+running into a "dangling else" situation.
+
+
+Note that comments should only be hoisted for loops and
+``if``, and not in ``else if`` or ``else``, where it would be unclear whether the comment
+belonged to the preceeding condition, or the ``else``.
+
+.. code-block:: c++
+
+  // Omit the braces, since the body is simple and clearly associated with the if.
+  if (isa<FunctionDecl>(D))
+    handleFunctionDecl(D);
+  else if (isa<VarDecl>(D))
+    handleVarDecl(D);
+  else {
+    // In this else case, it is necessary that we explain the situation with this
+    // surprisingly long comment, so it would be unclear without the braces whether
+    // the following statement is in the scope of the else.
+    handleOtherDecl(D);
+  }
+
+  // This should also omit braces.  The for loop contains only a single statement,
+  // so it shouldn't have braces.  The if also only contains a single statement (the
+  // for loop), so it also should omit braces.
+  if (isa<FunctionDecl>(D))
+    for(auto *A : D.attrs())
+      handleAttr(A);
+
+
 See Also
 ========