[clang-format] C++11 braced lists should respect the SpacesInParentheses setting

[mitchell-stellar]

Summary

According to the clang-format documentation, "Fundamentally, C++11 braced lists are formatted exactly like function calls would be formatted in their place. If the braced list follows a name (e.g. a type or variable name), clang-format formats as if the {} were the parentheses of a function call with that name."

This patch furthers the treatment of C++11 braced list braces as parentheses by respecting the SpacesInParentheses setting.

[MyDeveloperDay]

Should we have SpacesInBraces? as a first class formatting option, given that we have?

SpaceBetweenBraces.SpacesInAngles = true;
SpaceBetweenBraces.SpacesInParentheses = true;
SpaceBetweenBraces.SpacesInSquareBrackets = true;

How would that interact with SpaceInEmptyParentheses

[mitchell-stellar]

I don't think that's a good idea, considering the fact that braces can mean different things in different contexts, and it would cause trouble for existing clang-format settings.

If a hypothetical SpacesInBraces were false by default, then you could have clang-format output something like:

[](int foo) {bar(foo);}

Currently, it is [](int foo) { bar(foo); }.

If a hypothetical SpacesInBraces were true by default, then you could have clang-format output something like:

vector<int> x{ 1, 2, 3, 4 };

Currently, it is vector<int> x{1, 2, 3, 4};.

This patch is minimally invasive and adheres to the existing documentation.

[mitchell-stellar]

Added additional unit tests to verify that the SpaceInEmptyParentheses setting is also correctly adhered to.

[MyDeveloperDay]

what if you have this

vector<int>{1, 2, 3};
new int[3]{1, 2, 3};
foo(int f);

but want this:

vector<int>{ 1, 2, 3 };
new int[3]{ 1, 2, 3 };
foo(int f);

wouldn't turning on SpacesInParentheses: true now mean you get

vector<int>{ 1, 2, 3 };
new int[3]{ 1, 2, 3 };
foo( int f );

so there is no way of still having foo(int f); without a space before the int and after the f?

Sorry did I miss something? Generally, I agree that the change is good for the braced case I just don't want to break everyones functions

[mitchell-stellar]

If you want spaces in your C++11 initializers and nothing else, then you don't want the Cpp11BracedListStyle setting enabled. Turning it off gives you spaces inside without affecting anything else.

[MyDeveloperDay]

understood, ok LGTM

[mitchell-stellar]

Thanks. Please commit on my behalf when you have a chance.

[MyDeveloperDay]

@mitchell-stellar I pulled the patch to commit it, but the unit tests fail, could you double-check them for me?

[       OK ] FormatTest.LayoutBraceInitializersInReturnStatement (13 ms)
[ RUN      ] FormatTest.LayoutCxx11BraceInitializers
C:/cygwin64/buildareas/clang/llvm-project/clang/unittests/Format/FormatTest.cpp(70): error:       Expected: Expected.str()
      Which is: "vector< int > x{ };"
To be equal to: format(Expected, Style)
      Which is: "vector< int > x{};"
Expected code is not stable
C:/cygwin64/buildareas/clang/llvm-project/clang/unittests/Format/FormatTest.cpp(72): error:       Expected: Expected.str()
      Which is: "vector< int > x{ };"
To be equal to: format(Code, Style)
      Which is: "vector< int > x{};"
C:/cygwin64/buildareas/clang/llvm-project/clang/unittests/Format/FormatTest.cpp(78): error:       Expected: Expected.str()
      Which is: "vector< int > x{ };"
To be equal to: format(test::messUp(Code), ObjCStyle)
      Which is: "vector< int > x{};"
[  FAILED  ] FormatTest.LayoutCxx11BraceInitializers (1024 ms)
[ RUN      ] FormatTest.FormatsBracedListsInColumnLayout

[mitchell-stellar]

You are correct. I'll work on this.

[mitchell-stellar]

Fixed empty parentheses unit test for C++11 braced initializer list.

[MyDeveloperDay]

LGTM and passes

[----------] Global test environment tear-down
[==========] 700 tests from 21 test cases ran. (67039 ms total)
[  PASSED  ] 700 tests.

cfe/trunk/lib/Format/TokenAnnotator.cpp

void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
   FormatToken *Current = Line.First->Next;
   bool InFunctionDecl = Line.MightBeFunctionDecl;
   while (Current) {
     if (isFunctionDeclarationName(*Current, Line))
       Current->Type = TT_FunctionDeclarationName;
     if (Current->is(TT_LineComment)) {
       if (Current->Previous->BlockKind == BK_BracedInit &&
           Current->Previous->opensScope())
-        Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
+        Current->SpacesRequiredBefore =
+            (Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
       else
         Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
 
       // If we find a trailing comment, iterate backwards to determine whether
       // it seems to relate to a specific parameter. If so, break before that
       // parameter to avoid changing the comment's meaning. E.g. don't move 'b'
       // to the previous line in:
       //   SomeFunction(a,
if (Left.is(Keywords.kw_assert) && Style.Language == FormatStyle::LK_Java)
     return true;
   if (Style.ObjCSpaceAfterProperty && Line.Type == LT_ObjCProperty &&
       Left.Tok.getObjCKeywordID() == tok::objc_property)
     return true;
   if (Right.is(tok::hashhash))
     return Left.is(tok::hash);
   if (Left.isOneOf(tok::hashhash, tok::hash))
     return Right.is(tok::hash);
-  if (Left.is(tok::l_paren) && Right.is(tok::r_paren))
+  if ((Left.is(tok::l_paren) && Right.is(tok::r_paren)) ||
+      (Left.is(tok::l_brace) && Left.BlockKind != BK_Block &&
+       Right.is(tok::r_brace) && Right.BlockKind != BK_Block))
     return Style.SpaceInEmptyParentheses;
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
     return (Right.is(TT_CastRParen) ||
             (Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen)))
                ? Style.SpacesInCStyleCastParentheses
                : Style.SpacesInParentheses;
   if (Right.isOneOf(tok::semi, tok::comma))
     return false;
if (Right.is(tok::l_square) &&
                      TT_StructuredBindingLSquare, TT_AttributeSquare) &&
       !Left.isOneOf(tok::numeric_constant, TT_DictLiteral))
     return false;
   if (Left.is(tok::l_brace) && Right.is(tok::r_brace))
     return !Left.Children.empty(); // No spaces in "{}".
   if ((Left.is(tok::l_brace) && Left.BlockKind != BK_Block) ||
       (Right.is(tok::r_brace) && Right.MatchingParen &&
        Right.MatchingParen->BlockKind != BK_Block))
-    return !Style.Cpp11BracedListStyle;
+    return Style.Cpp11BracedListStyle ? Style.SpacesInParentheses : true;
   if (Left.is(TT_BlockComment))
     // No whitespace in x(/*foo=*/1), except for JavaScript.
     return Style.Language == FormatStyle::LK_JavaScript ||
            !Left.TokenText.endswith("=*/");
   if (Right.is(tok::l_paren)) {
     if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
         (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
       return true;

cfe/trunk/unittests/Format/FormatTest.cpp

verifyFormat("const std::unordered_map<std::string, int> MyHashTable{\n"
                "  { \"ccccccccccccccccccccc\", 2 }\n"
                "};",
                ExtraSpaces);
 
   FormatStyle SpaceBeforeBrace = getLLVMStyle();
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector<int> x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+               "                 1, 2, 3, 4 };",
+               SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+            "    1, 2, 3, 4 };",
+            format("vector<int>x{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+            "                 3, 4 };",
+            format("vector<int>x{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+            "    1,\n"
+            "    2,\n"
+            "    3,\n"
+            "    4,\n"
+            "};",
+            format("vector<int>x{1,2,3,4,};", SpaceBetweenBraces));
+  verifyFormat("vector< int > x{};", SpaceBetweenBraces);
+  SpaceBetweenBraces.SpaceInEmptyParentheses = true;
+  verifyFormat("vector< int > x{ };", SpaceBetweenBraces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
   verifyFormat("vector<int> x = {1, 22, 333, 4444, 55555, 666666, 7777777,\n"
                "                 1, 22, 333, 4444, 55555, 666666, 7777777,\n"
                "                 1, 22, 333, 4444, 55555, 666666, 7777777,\n"
                "                 1, 22, 333, 4444, 55555, 666666, 7777777,\n"
                "                 1, 22, 333, 4444, 55555, 666666, 7777777,\n"