[PATCH] D82594: Clarify a bit the guideline on omitting braces, including more examples (NFC)

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini marked an inline comment as done.
mehdi_amini added inline comments.


================
Comment at: llvm/docs/CodingStandards.rst:1657
+  if (auto *D = dyn_cast<FunctionDecl>(D)) {
+    if(shouldProcess(D))
+      handleVarDecl(D);
----------------
MaskRay wrote:
> lattner wrote:
> > mehdi_amini wrote:
> > > lattner wrote:
> > > > MaskRay wrote:
> > > > > Add a space after `if`
> > > > and after the for.  Also, I'd recommend using size_t instead of 'int', and != instead of <.  
> > > Why the `size_t` here? I thought best practice was rather to write for loops with `int`?
> > size_t is technically correct and the best type to use.  LLVM historically has used 'unsigned' for most counted loops (this was my fault) which isn't great for induction variable optimizations on 64-bit machines.  "int" is better for 64-bit machines because UB-on-overflow permits the important transformations, but this is technically incorrect and there is no obvious reason to prefer it.
> > "int" is better for 64-bit machines because UB-on-overflow permits the important transformations, but this is technically incorrect and there is no obvious reason to prefer it.
> 
> An unsigned index might be relevant for 16-bit computers. When we come to 32-bit and 64-bit era, ability to index more than half of the memory seems pretty irrelevant... Defined modulo 2 behavior impedes optimization and breeds bugs like `for (size_t i = 0; i < a.size() - 1; ++i)`. Using int is fine, though I acknowledge that unsigned has been used a lot in LLVM's code base. C++20 even introduced std::ssize to make signed indexes more of "first-class" (I believe it is http://wg21.link/p1227r2 )
> size_t is technically correct and the best type to use. 

How so? This contradicts most of what I read in "recent" C++ talk / publications, I summarized it here: http://lists.llvm.org/pipermail/llvm-dev/2019-June/132890.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82594