[PATCH] D126512: [Docs] Clarify the guideline on omitting braces

Owen Pan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 31 10:52:27 PDT 2022


owenpan added inline comments.


================
Comment at: llvm/docs/CodingStandards.rst:1633
-      if (shouldProcessAttr(A))
-        handleAttr(A);
   }
----------------
erichkeane wrote:
> owenpan wrote:
> > erichkeane wrote:
> > > owenpan wrote:
> > > > mehdi_amini wrote:
> > > > > owenpan wrote:
> > > > > > mehdi_amini wrote:
> > > > > > > This example shows how this revision goes beyond do/while.
> > > > > > This example was very similar to the one starting at line 1662. After taking out one level of nesting here, it won't seem redundant to the "Use braces on the outer block because there are more than two levels of nesting" example below.
> > > > > I rather keep this example though if we don’t have a good reason to remove it
> > > > You would use the braces on the outer `if` in the original example **regardless** whether there was a potential dangling `else` situation because it had two levels of nesting. The revision removes the duplication. (In clang-format unit tests, this example doesn't really test the rule because of the multilevel nesting one below.)
> > > Where in the prose do you see the '2 levels of nesting rule'?  I don't see anything like that in the rules...
> > > Where in the prose do you see the '2 levels of nesting rule'?  I don't see anything like that in the rules...
> > 
> > Line 1662:
> > ```
> > // Use braces on the outer block because there are more than two levels of nesting.
> > ```
> Strange, that one is added JUST in example?!  I don't think we've been enforcing that anywhere.  I wonder if we treat examples as normative in this document...
> Strange, that one is added JUST in example?!  I don't think we've been enforcing that anywhere.  I wonder if we treat examples as normative in this document...

Yeah, and there are a few more that are only in the examples but not in the prose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126512



More information about the llvm-commits mailing list