[clang-format] Add SpaceBeforeCpp11BracedList option.

[rkirsling]

Summary

WebKit C++ style for object initialization is as follows:

Foo foo { bar };

Yet using clang-format -style=webkit changes this to:

Foo foo{ bar };

As there is no existing combination of rules that will ensure a space before a braced list in this fashion, this patch adds a new SpaceBeforeCpp11BracedList rule.

[klimek]

Is this written down somewhere? https://webkit.org/code-style-guidelines/ doesn't seem to mention it.

[rkirsling]

In D46024#1079390, @klimek wrote:

Is this written down somewhere? https://webkit.org/code-style-guidelines/ doesn't seem to mention it.

I agree that it really ought to be mentioned there—I'll try to bring that up with the original authors of that document.

It is enforced by the style checker (though the rule is not specific to braced initialization):
https://github.com/WebKit/webkit/blob/master/Tools/Scripts/webkitpy/style/checkers/cpp.py#L1972-L1978

Statistically speaking, when grepping the Source directory (excluding Source/ThirdParty) for *.h and *.cpp files:

• with space (\w+\s\{.*\};) has 11544 matches across 2744 files
• without space (\w+\{.*\};) has 29 matches across 10 files

[rkirsling]

Guidelines page has been updated (https://trac.webkit.org/changeset/231085), though it may take a bit for the website to update.

[rkirsling]

Rule has been published:
https://webkit.org/code-style-guidelines/#spacing-braced-init

Hopefully that suffices for motivation. :)

[jfb]

On the WebKit side this lgtm. Let's leave some time for clang-format folks to chime in.

[rkirsling]

Any further commentary? :)

[rkirsling]

Updated patch to resolve conflict and include full diff context.

[rkirsling]

If there are no objections to this change, may I request a commit based on the approval that @jfb provided?

[rkirsling]

@klimek In our IRC discussion yesterday, I know you expressed disapproval of WebKit's choice, but given its reality, am I correct in concluding that this can be landed?

[rkirsling]

Resolved another rebase conflict to keep this patch mergeable.

[rkirsling]

FWIW, please note that this space-before-brace style is not specific to WebKit; CppCoreGuidelines exhibits it as well:
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initializer-syntax

[jfb]

In D46024#1121242, @rkirsling wrote:

FWIW, please note that this space-before-brace style is not specific to WebKit; CppCoreGuidelines exhibits it as well:
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initializer-syntax

The main point of clang-format, for me , is to not argue about style. Can we... not argue about WebKit's and others' choice? :)

[hans]

In D46024#1121242, @rkirsling wrote:

FWIW, please note that this space-before-brace style is not specific to WebKit; CppCoreGuidelines exhibits it as well:
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initializer-syntax

This and WebKit's style seem like compelling arguments to support this option.

klimek, djasper: Do you have any objections against landing this?

[klimek]

In D46024#1129350, @hans wrote:

In D46024#1121242, @rkirsling wrote:

FWIW, please note that this space-before-brace style is not specific to WebKit; CppCoreGuidelines exhibits it as well:
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initializer-syntax

This and WebKit's style seem like compelling arguments to support this option.

klimek, djasper: Do you have any objections against landing this?

Agreed.
Generally LG minus that I'd significantly reduce the number of test cases :)

unittests/Format/FormatTest.cpp

6980 ↗

(On Diff #149395)

There are super many redundant test cases here - I don't think we need to test that brace init detection works here, again.
I think given the code change we basically need 2 tests:
one where the previous opens a scope, and one where it doesn't.

[rkirsling]

Addressed feedback—thank you for the review!

[hans]

Ross, do you have commit access or do you need someone to commit this for you?

[rkirsling]

I'll need someone to commit. Thanks!

cfe/trunk/docs/ClangFormatStyleOptions.rst

**SpaceBeforeAssignmentOperators** (``bool``)
   If ``false``, spaces will be removed before assignment operators.
 
   .. code-block:: c++
 
      true:                                  false:
      int a = 5;                     vs.     int a=5;
      a += 42                                a+=42;
 
+**SpaceBeforeCpp11BracedList** (``bool``)
+  If ``true``, a space will be inserted before a C++11 braced list
+  used to initialize an object (after the preceding identifier or type).
+
+  .. code-block:: c++
+
+     true:                                  false:
+     Foo foo { bar };               vs.     Foo foo{ bar };
+     Foo {};                                Foo{};
+     vector<int> { 1, 2, 3 };               vector<int>{ 1, 2, 3 };
+     new int[3] { 1, 2, 3 };                new int[3]{ 1, 2, 3 };
+
 **SpaceBeforeCtorInitializerColon** (``bool``)
   If ``false``, spaces will be removed before constructor initializer
   colon.
 
   .. code-block:: c++
 
      true:                                  false:
      Foo::Foo() : a(a) {}                   Foo::Foo(): a(a) {}

cfe/trunk/include/clang/Format/Format.h

   /// If ``false``, spaces will be removed before assignment operators.
   /// \code
   ///    true:                                  false:
   ///    int a = 5;                     vs.     int a=5;
   ///    a += 42                                a+=42;
   /// \endcode
   bool SpaceBeforeAssignmentOperators;
 
+  /// If ``true``, a space will be inserted before a C++11 braced list
+  /// used to initialize an object (after the preceding identifier or type).
+  /// \code
+  ///    true:                                  false:
+  ///    Foo foo { bar };               vs.     Foo foo{ bar };
+  ///    Foo {};                                Foo{};
+  ///    vector<int> { 1, 2, 3 };               vector<int>{ 1, 2, 3 };
+  ///    new int[3] { 1, 2, 3 };                new int[3]{ 1, 2, 3 };
+  /// \endcode
+  bool SpaceBeforeCpp11BracedList;
+
   /// If ``false``, spaces will be removed before constructor initializer
   /// colon.
   /// \code
   ///    true:                                  false:
   ///    Foo::Foo() : a(a) {}                   Foo::Foo(): a(a) {}
   /// \endcode
   bool SpaceBeforeCtorInitializerColon;
 
return AccessModifierOffset == R.AccessModifierOffset &&
            PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
            PenaltyBreakTemplateDeclaration ==
                R.PenaltyBreakTemplateDeclaration &&
            PointerAlignment == R.PointerAlignment &&
            RawStringFormats == R.RawStringFormats &&
            SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
            SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
            SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
+           SpaceBeforeCpp11BracedList == R.SpaceBeforeCpp11BracedList &&
            SpaceBeforeCtorInitializerColon ==
                R.SpaceBeforeCtorInitializerColon &&
            SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
            SpaceBeforeParens == R.SpaceBeforeParens &&
            SpaceBeforeRangeBasedForLoopColon ==
                R.SpaceBeforeRangeBasedForLoopColon &&
            SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
            SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&

cfe/trunk/lib/Format/Format.cpp

static void mapping(IO &IO, FormatStyle &Style) {
     IO.mapOptional("ReflowComments", Style.ReflowComments);
     IO.mapOptional("SortIncludes", Style.SortIncludes);
     IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
     IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
     IO.mapOptional("SpaceAfterTemplateKeyword",
                    Style.SpaceAfterTemplateKeyword);
     IO.mapOptional("SpaceBeforeAssignmentOperators",
                    Style.SpaceBeforeAssignmentOperators);
+    IO.mapOptional("SpaceBeforeCpp11BracedList",
+                   Style.SpaceBeforeCpp11BracedList);
     IO.mapOptional("SpaceBeforeCtorInitializerColon",
                    Style.SpaceBeforeCtorInitializerColon);
     IO.mapOptional("SpaceBeforeInheritanceColon",
                    Style.SpaceBeforeInheritanceColon);
     IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
     IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
                    Style.SpaceBeforeRangeBasedForLoopColon);
     IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses);
FormatStyle getLLVMStyle() {
   LLVMStyle.SpacesInCStyleCastParentheses = false;
   LLVMStyle.SpaceAfterCStyleCast = false;
   LLVMStyle.SpaceAfterTemplateKeyword = true;
   LLVMStyle.SpaceBeforeCtorInitializerColon = true;
   LLVMStyle.SpaceBeforeInheritanceColon = true;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
   LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
+  LLVMStyle.SpaceBeforeCpp11BracedList = false;
   LLVMStyle.SpacesInAngles = false;
 
   LLVMStyle.PenaltyBreakAssignment = prec::Assignment;
   LLVMStyle.PenaltyBreakComment = 300;
   LLVMStyle.PenaltyBreakFirstLessLess = 120;
   LLVMStyle.PenaltyBreakString = 1000;
   LLVMStyle.PenaltyExcessCharacter = 1000000;
   LLVMStyle.PenaltyReturnTypeOnItsOwnLine = 60;
FormatStyle getWebKitStyle() {
   Style.Cpp11BracedListStyle = false;
   Style.ColumnLimit = 0;
   Style.FixNamespaceComments = false;
   Style.IndentWidth = 4;
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   Style.ObjCBlockIndentWidth = 4;
   Style.ObjCSpaceAfterProperty = true;
   Style.PointerAlignment = FormatStyle::PAS_Left;
+  Style.SpaceBeforeCpp11BracedList = true;
   return Style;
 }
 
 FormatStyle getGNUStyle() {
   FormatStyle Style = getLLVMStyle();
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;

cfe/trunk/lib/Format/TokenAnnotator.cpp

 bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
                                          const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
   if (Right.Tok.getIdentifierInfo() && Left.Tok.getIdentifierInfo())
     return true; // Never ever merge two identifiers.
   if (Style.isCpp()) {
     if (Left.is(tok::kw_operator))
       return Right.is(tok::coloncolon);
+    if (Right.is(tok::l_brace) && Right.BlockKind == BK_BracedInit &&
+        !Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
+      return true;
   } else if (Style.Language == FormatStyle::LK_Proto ||
              Style.Language == FormatStyle::LK_TextProto) {
     if (Right.is(tok::period) &&
         Left.isOneOf(Keywords.kw_optional, Keywords.kw_required,
                      Keywords.kw_repeated, Keywords.kw_extend))
       return true;
     if (Right.is(tok::l_paren) &&
         Left.isOneOf(Keywords.kw_returns, Keywords.kw_option))

cfe/trunk/unittests/Format/FormatTest.cpp

verifyFormat("const std::unordered_map<std::string, int> MyHashTable = {\n"
                "};",
                ExtraSpaces);
   verifyFormat("const std::unordered_map<std::string, int> MyHashTable{\n"
                "  { \"aaaaaaaaaaaaaaaaaaaaa\", 0 },\n"
                "  { \"bbbbbbbbbbbbbbbbbbbbb\", 1 },\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);
 }
 
 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"
TEST_F(FormatTest, ParsesConfigurationBools) {
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInAngles);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
   CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
   CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
+  CHECK_PARSE_BOOL(SpaceBeforeCpp11BracedList);
   CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
   CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
   CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
 
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterControlStatement);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterEnum);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterFunction);