hubert.reinterpretcast added inline comments.
================ Comment at: llvm/docs/CodingStandards.rst:1578 +unnecessary and otherwise meaningless code. Braces should be used however +in cases where it significantly improves readability, such as when the single +statement is accompanied by a comment that loses its meaning if hoisted above the `if` ---------------- erichkeane wrote: > hubert.reinterpretcast wrote: > > I think "navigability" is also negatively affected by omission of braces. > > This seems to be an aspect of readability this is not always considered. It > > tends to be easier to consume code in an editor when placing a cursor on a > > brace highlights the matching brace. If a reviewer in a web interface > > needed to scroll or "draw a line" to where a loop or if/else chain starts > > when reaching the end of a block, then the lack of braces is harmful. This > > would especially be the case if the code was such that having comments > > after the brace would be helpful. > > > > The use of braces to proactively avoid running into the dangling-else > > problem should also be permitted or even encouraged. > > > > Replacing the list of cases where braces help readability with a list of > > cases where omitting braces are harmful may help. We can then enforce > > braces for some classes of harmful brace-omission and permit braces for > > other classes. > > > > Examples of "mild" harmful cases can then include mixing of braced and > > non-braced blocks in an if/else chain. > Can you suggest an alternate wording here? I'm not sure how to capture what > you're saying. Suggested wording: ``` 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 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. 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. ``` ================ Comment at: llvm/docs/CodingStandards.rst:1599 + // surprisingly long comment, so it would be unclear without the braces whether + // the following statement is in the scope of the else. + handleOtherDecl(D); ---------------- I believe my comment was misunderstood. I meant that this part was fine (and there should be something to distinguish between the keywords and the English words). What I meant was that the change implied by another comment of mine (https://reviews.llvm.org/D80947?id=267696#inline-744103) did not apply here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80947/new/ https://reviews.llvm.org/D80947 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
