mirror of
https://github.com/RPCS3/llvm-mirror.git
synced 2024-11-22 18:54:02 +01:00
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
This commit is contained in:
parent
14ef44b9eb
commit
0080e83362
@ -1575,28 +1575,29 @@ 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.
|
||||
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 difficult 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.
|
||||
|
||||
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 difficult 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.
|
||||
``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.
|
||||
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.
|
||||
@ -1604,20 +1605,67 @@ belonged to the preceeding condition, or the ``else``.
|
||||
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.
|
||||
|
||||
|
||||
// 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);
|
||||
}
|
||||
|
||||
// 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.
|
||||
// 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 `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 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
|
||||
========
|
||||
|
Loading…
Reference in New Issue
Block a user