[llvm] 140c296 - Clarify a bit the guideline on omitting braces, including more examples (NFC)

[Mehdi Amini via llvm-commits]

Author: Mehdi Amini
Date: 2020-07-15T21:11:30Z
New Revision: 140c296ef5144136a00b166384c752c37a176879

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

LOG: Clarify a bit the guideline on omitting braces, including more examples (NFC)

Like most readability rules, it isn't absolute and there is a matter of taste
to it. I think more recent part of the project may be more consistent in the
current application of the guideline. I suspect sources like
mlir/lib/Dialect/StandardOps/IR/Ops.cpp may be examples of this at the moment.

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

Added: 
    

Modified: 
    llvm/docs/CodingStandards.rst

Removed: 
    


################################################################################
diff  --git a/llvm/docs/CodingStandards.rst b/llvm/docs/CodingStandards.rst
index 99fb6af02a28..a9884cd9f3b5 100644
--- a/llvm/docs/CodingStandards.rst
+++ b/llvm/docs/CodingStandards.rst
@@ -1575,27 +1575,28 @@ 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.
+When writing the body of an ``if``, ``else``, or loop statement, we prefer to
+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.
+
+We consider that readability is harmed when omitting the brace in the presence
+of a single statement that is accompanied by a comment (assuming the comment
+can't be hoisted above the ``if`` or loop statement, see below).
+Similarly, braces should be used when a 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.
 
-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.
+This list is not exhaustive, for example, readability is also harmed if an
+``if``/``else`` chain does not use braced bodies for either all or none of its
+members, with complex conditionals, deep nesting, etc. The examples below
+intend to provide some guidelines.
 
+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++
 
@@ -1604,20 +1605,67 @@ belonged to the preceeding condition, or the ``else``.
     handleFunctionDecl(D);
   else if (isa<VarDecl>(D))
     handleVarDecl(D);
-  else {
+
+
+  // Here we document the condition itself and not the body.
+  if (isa<VarDecl>(D)) {
+    // 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 `if`.
+    // Because the condition is documented, we can't really hoist this
+    // comment that applies to the body above the if.
+    handleOtherDecl(D);
+  }
+
+  // Use braces on the outer `if` to avoid a potential dangling else situation.
+  if (isa<VarDecl>(D)) {
+    for (auto *A : D.attrs())
+      if (shouldProcessAttr(A))
+        handleAttr(A);
+  }
+
+  // Use braces for the `if` block to keep it uniform with the else block.
+  if (isa<FunctionDecl>(D)) {
+    handleFunctionDecl(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.
+    // the following statement is in the scope of the `if`.
     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.
+  // 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 simple
+  // statement (the for loop), so it also should omit braces.
   if (isa<FunctionDecl>(D))
     for (auto *A : D.attrs())
       handleAttr(A);
 
+  // Use braces for the outer `if` since the nested `for` is braced.
+  if (isa<FunctionDecl>(D)) {
+    for (auto *A : D.attrs()) {
+      // In this for loop body, it is necessary that we explain the situation
+      // with this surprisingly long comment, forcing braces on the `for` block.
+      handleAttr(A);
+    }
+  }
+
+  // Use braces on the outer block because there are more than two levels of nesting.
+  if (isa<FunctionDecl>(D)) {
+    for (auto *A : D.attrs())
+      for (ssize_t i : llvm::seq<ssize_t>(count))
+         handleAttrOnDecl(D, A, i);
+  }
+
+  // Use braces on the outer block because of a nested `if`, otherwise the
+  // compiler would warn: `add explicit braces to avoid dangling else`
+  if (auto *D = dyn_cast<FunctionDecl>(D)) {
+    if (shouldProcess(D))
+      handleVarDecl(D);
+    else
+      markAsIgnored(D);
+  }
+
 
 See Also
 ========