Update the coding standards to provide some guidance for a few constructs in C++11

[chandlerc]

Summary

When looking at switching LLVM's code to build in C++11 mode, most of the coding standard applies just fine. Most of the new features we know how to use, or we will learn through code review. But a few of the more mechanical bits would be easy to diverge wildly and result in inconsistency and confusion. In some cases, there are features that are fairly important to be used in a particular way to keep the code maintainable.

I talked to Richard Smith, Doug Gregor, and John McCall (as some of the more language oriented folks in the community), and with their help I came up with these additions to the coding standards. I'd like to get some broader input on these, but ideally I'd like to paint these bikesheds pretty quickly, put something in place, and revisit them if these guidelines don't really work out in practice.

[jdennett]

This looks helpful, I have just a couple of questions about how the sun might strike the southern aspect of the bikeshed in summer.

docs/CodingStandards.rst

513

Meaning lambdas, or...? (I"m not sure this warrants any editing, but as a reader my first reaction to this was "I wonder what that's talking about".)

667

Does use of a constructor taking a std::initializer_list<T> parameter count as "notionally equivalent" to aggregate initialization, or is it under the general "Do not use these...if you care that you're calling some *particular* constructor" case? I could make an argument for either.

Maybe the easiest way to document that is with an example indicating whether
std::vector<int> Very SmallPrimes = { 2, 3, 5 };
is permitted or not.

(While I do have a preferred answer, I'm not here to lobby for it, just to ask what the intent is. Lobbying can happen later. Much later.)

[lhames]

Seconding jdennett's comments.

Other than that, LGTM.

[chandlerc]

docs/CodingStandards.rst

513

Yep, just meaning lambdas.

667

Hmm. I'm a bit torn on this one.

I am OK allowing the variable syntax you give, but I would like to require that if you are creating a temporary, you do something like:

std::vector<int>({1, 2})

I would also be OK with requiring the same syntax for variable declarations:

std::vector<int> x({1, 2});

[silvas]

docs/CodingStandards.rst

490–502

Sort of random, but it seems like clang-format doesn't get this quite right. Maybe we should add something to the coding standard describing how to work around/with clang-format to get desirable results?

E.g.

First attempt, no coercion:

int foo() {
  dyn_switch(V->stripPointerCasts(), [](PHINode *PN) {
                                       // process phis...
                                     },
             [](SelectInst *SI) {
               // process selects...
             },
             [](LoadInst *LI) {
               // process loads...
             },
             [](AllocaInst *AI) {
    // process allocas...
  });
}

Adding one comment:

int foo() {
  dyn_switch(V->stripPointerCasts(), //
             [](PHINode *PN) {
               // process phis...
             },
             [](SelectInst *SI) {
               // process selects...
             },
             [](LoadInst *LI) {
               // process loads...
             },
             [](AllocaInst *AI) {
    // process allocas...
  });
}

And then, for the "hanging" last lambda:

int foo() {
  dyn_switch(V->stripPointerCasts(), //
             [](PHINode *PN) {
               // process phis...
             },
             [](SelectInst *SI) {
               // process selects...
             },
             [](LoadInst *LI) {
               // process loads...
             },
             [](AllocaInst *AI) {
               // process allocas...
             }//
             );
}

(The lack of space before that line comment is PR19017).

[chandlerc]

docs/CodingStandards.rst

490–502

Bleh. Lame. I've filed PR19021 for this. I think we should just fix clang-format here.

[Matt]

docs/CodingStandards.rst

667

Hi!

Out of curiosity, I'm wondering about the upsides/downsides of following the Scott Meyers' advice in this context.
// "Develop the habit of using brace initialization without “=“.
In: "Overview of the New C++ (C++11)"; http://www.aristeia.com/C++11.html

In "Uniform Initialization Syntax" he provides an example where the alternative would fail:

const float * pData = new const float[3] = { 1.f, 2.f, 3.f }; // error
const float * pData = new const float[3] { 1.f, 2.f, 3.f }; // ok

Similarly, another example shows that T var = expr syntax can’t call explicit constructors; adopting from the example (slightly simplifying, changing Widget to struct) therein:

struct Widget
{
	explicit Widget(int);
};
Widget w1(10); // okay, direct init: explicit ctor callable
Widget w2{10}; // ditto
Widget w3 = 10; // error! copy init: explicit ctor not callable
Widget w4 = {10}; // ditto

Or, is disallowing the above the actual intent?

docs/CodingStandards.rst

 indentation, **DO NOT** do that in the middle of a chunk of code with two spaces
 of indentation.  Also, do not reindent a whole source file: it makes for
 incredible diffs that are absolutely worthless.
 
 Indent Code Consistently
 ^^^^^^^^^^^^^^^^^^^^^^^^
 
 Okay, in your first year of programming you were told that indentation is
-important.  If you didn't believe and internalize this then, now is the time.
-Just do it.
+important. If you didn't believe and internalize this then, now is the time.
+Just do it. With the introduction of C++11, there are some new formatting
+challenges that merit some suggestions to help have consistent, maintainable,
+and tool-friendly formatting and indentation.
 
-Compiler Issues
----------------
+Format Lambdas Like Blocks Of Code
+""""""""""""""""""""""""""""""""""
+
+When formatting a multi-line lambda, format it like a block of code, that's
+what it is. If there is only one multi-line lambda in a statement, and there
+are no expressions lexically after it in the statement, drop the indent to the
+standard two space indent for a block of code, as if it were an if-block opened
+by the preceding part of the statement:
+
+.. code-block:: c++
+
+  std::sort(foo.begin(), foo.end(), [&](Foo a, Foo b) -> bool {
+    if (a.blah < b.blah)
+      return true;
+    if (a.baz < b.baz)
+      return true;
+    return a.bam < b.bam;
+  });
+
+If there are multiple multi-line lambdas in a statement, or there is anything
+interesting after the lambda in the statement, indent the block two spaces from
+the indent of the ``[]``:
+
+.. code-block:: c++
+
+  dyn_switch(V->stripPointerCasts(),
+             [] (PHINode *PN) {
+               // process phis...
+             },
+             [] (SelectInst *SI) {
+               // process selects...
+             },
+             [] (LoadInst *LI) {
+               // process loads...
+             },
+             [] (AllocaInst *AI) {
+               // process allocas...
+             });
+
+Braced Initializer Lists
+""""""""""""""""""""""""
+
+With C++11, there are significantly more uses of braced lists to perform
+initialization. These allow you to easily construct aggregate temporaries in
+expressions among other niceness. They now have a natural way of ending up
+nested within each other and within function calls in order to build up
+aggregates (such as option structs) from local variables. To make matters
+worse, we also have many more uses of braces in an expression context that are
+*not* performing initialization.
+
+The historically common formatting of braced initialization of aggregate
+variables does not mix cleanly with deep nesting, general expression contexts,
+function arguments, and lambdas. We suggest new code use a simple rule for
+formatting braced initialization lists: act as-if the braces were parentheses
+in a function call. The formatting rules exactly match those already well
+understood for formatting nested function calls. Examples:
+
+.. code-block:: c++
+
+  foo({a, b, c}, {1, 2, 3});
+
+  llvm::Constant *Mask[] = {
+      llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 0),
+      llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 1),
+      llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 2)};
+
+This formatting scheme also makes it particularly easy to get predictable,
+consistent, and automatic formatting with tools like `Clang Format`_.
+
+.. _Clang Format: http://clang.llvm.org/docs/ClangFormat.html
+
+Language and Compiler Issues
+----------------------------
 
 Treat Compiler Warnings Like Errors
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 If your code has compiler warnings in it, something is wrong --- you aren't
 casting values correctly, you have "questionable" constructs in your code, or
 you are doing something legitimately wrong.  Compiler warnings can cover up
 legitimate errors in output and make dealing with a translation unit difficult.

 different symbols based on whether ``class`` or ``struct`` was used to declare
 the symbol.  This can lead to problems at link time.
 
 So, the rule for LLVM is to always use the ``class`` keyword, unless **all**
 members are public and the type is a C++ `POD
 <http://en.wikipedia.org/wiki/Plain_old_data_structure>`_ type, in which case
 ``struct`` is allowed.
 
+Do not use Braced Initializer Lists to Call a Constructor
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+In C++11 there is a "generalized initialization syntax" which allows calling
+constructors using braced initializer lists. Do not use these to call
+constructors with any interesting logic or if you care that you're calling some
+*particular* constructor. Those should look like function calls using
+parentheses rather than like aggregate initialization. Similarly, if you need
+to explicitly name the type and call its constructor to create a temporary,
+don't use a braced initializer list. Instead, use a braced initializer list
+(without any type for temporaries) when doing aggregate initialization or
+something notionally equivalent. Examples:
+
+.. code-block:: c++
+
+  class Foo {
+  public:
+    // Construct a Foo by reading data from the disk in the whizbang format, ...
+    Foo(std::string filename);
+
+    // Construct a Foo by looking up the Nth element of some global data ...
+    Foo(int N);
+
+    // ...
+  };
+
+  // The Foo constructor call is very deliberate, no braces.
+  std::fill(foo.begin(), foo.end(), Foo("name"));
+
+  // The pair is just being constructed like an aggregate, use braces.
+  bar_map.insert({my_key, my_value});
+
+If you use a braced initializer list when initializing a variable, use an equals before the open curly brace:
+
+.. code-block:: c++
+
+  int data[] = {0, 1, 2, 3};
+
+Use ``auto`` Type Deduction to Make Code More Readable
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Some are advocating a policy of "almost always ``auto``" in C++11, however LLVM
+uses a more moderate stance. Use ``auto`` if and only if it makes the code more
+readable or easier to maintain. Don't "almost always" use ``auto``, but do use
+``auto`` with initializers like ``cast<Foo>(...)`` or other places where the
+type is already obvious and clear.
+
 Style Issues
 ============
 
 The High-Level Issues
 ---------------------
 
 A Public Header File **is** a Module
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^