clang-format: Do not binpack initialization lists

[Typz]

Summary

This patch tries to avoid binpacking when initializing lists/arrays, to allow things like:

static int types[] = {
    registerType1(),
    registerType2(),
    registerType3(),
};
std::map<int, std::string> x = {
    { 0, "foo fjakfjaklf kljj" },
    { 1, "bar fjakfjaklf kljj" },
    { 2, "stuff fjakfjaklf kljj" },
};

This is similar to how dictionnaries are formatted, and actually corresponds to the same conditions: when initializing a container (and not just 'calling' a constructor).

Such formatting involves 2 things:

• Line breaks around the content of the block. This can be forced by adding a comma or comment after the last element
• Elements should not be binpacked

This patch considers the block is an initializer list if it either ends with a comma, or follows an assignment, which seems to provide a sensible approximation.

[Typz]

This patch is probably not complete, though it works fine in all situations I could think of: nested initializers, "short" statement (properly merged), column layout is still performed when needed...

static int types[] = {
    SourcePrivate::registerTypes(),
    registerEnum<MediaItemType>(),
    registerEnum<MediaItemFlag>(),
};
static int types[] = {registerTypes(),
                      registerEnum<MediaItemType>(),
                      registerEnum<MediaItemFlag>()};
static int types[]{
    SourcePrivate::registerTypes(),
    registerEnum<MediaItemType>(),
    registerEnum<MediaItemFlag>(),
};
static int types[]{SourcePrivate::registerTypes(),
                   registerEnum<MediaItemType>(),
                   registerEnum<MediaItemFlag>()};
static int types[]{0, 1, 2};
static int types[] = {0, 1, 2};
static int types[] = {aaaaaaaaaaa,  bbbbbbbbbb,   cccccccccccc,
                      dddddddddddd, eeeeeeeeeee,  fffffffffff,
                      gggggggggg,   hhhhhhhhhhhh, iiiiiiiiii};
static int types[] = {
    aaaaaaaaaaa, bbbbbbbbbb, cccccccccccc, dddddddddddd, eeeeeeeeeee,
    fffffffffff, gggggggggg, hhhhhhhhhhhh, iiiiiiiiii,
};
std::map<int, std::string> x = {
    {0, "foo fjakfjaklf kljj"},
    {1, "bar fjakfjaklf kljj"},
    {2, "stuff fjakfjaklf kljj"},
};

I have seen 2 problems so far, but they do not seem to be related:

• indent is performed using continuationIndentWidth, while I think IndentWidth may be more appropriate
• When force-wrapping due to comma, all the content may be merged afterwards, leading to some strange layout. In this case, I think it should either not merge the content at all, or merge the braces as well:

static int types[] = {
    0, 1, 2
};

But I would need some feedback:

• Would this be an interesting improvement?
• Should this come with an option, or should this be the new behavior?
• Any case in particular you think will break?
• Technically, is this the right approach, or should I introduce a new TokenType and try to determine it in TokenAnnotator? This may help factorize some code from TokenAnnotator (which adds the forced line breaks) and ContinuationIndenter (which prevents BinPacking) and possibly UnwrappedLineFormatter (to fix the issue where the items get merged but not the braces)
• Should I also update TokenAnalyser to force wrap around the braces after assignment, like what happens when there is a comma at the end?

lib/Format/ContinuationIndenter.cpp

1007 ↗

(On Diff #102670)

I don't really understand what this "TT_ArrayInitializerLSquare" case is...

[malcolm.parsons]

Some people write

auto x = std::map<int, std::string>{
    { 0, "foo fjakfjaklf kljj" },
    { 1, "bar fjakfjaklf kljj" },
    { 2, "stuff fjakfjaklf kljj" },
};

[Typz]

In D34238#781224, @malcolm.parsons wrote:

Some people write

auto x = std::map<int, std::string>{
    { 0, "foo fjakfjaklf kljj" },
    { 1, "bar fjakfjaklf kljj" },
    { 2, "stuff fjakfjaklf kljj" },
};

This case (and other way std::map<> x =) actually already worked, as long it ends with a comma: I suppose the braces at the beginning of each item already caused a line-break.

[djasper]

I am fine not bin-packing when the last element has a trailing comma. But lets not special case assignments.

[Typz]

fix unit tests

[Typz]

remove special case after assignment

[Typz]

fix indent

[Typz]

Fix case where the content fits on a line, by wrapping after each comma, like this:

static int types[] = {
        0,
        1,
        2,
};

[Typz]

Fix indentation

[Typz]

ping?

[djasper]

Looks good. Thank you!

cfe/trunk/lib/Format/ContinuationIndenter.cpp

if (Current.isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) ||
     } else {
       NewIndent = State.Stack.back().LastSpace + Style.ContinuationIndentWidth;
     }
     const FormatToken *NextNoComment = Current.getNextNonComment();
     bool EndsInComma = Current.MatchingParen &&
                        Current.MatchingParen->Previous &&
                        Current.MatchingParen->Previous->is(tok::comma);
     AvoidBinPacking =
-        (Current.is(TT_ArrayInitializerLSquare) && EndsInComma) ||
-        Current.is(TT_DictLiteral) ||
+        EndsInComma || Current.is(TT_DictLiteral) ||
         Style.Language == FormatStyle::LK_Proto || !Style.BinPackArguments ||
         (NextNoComment &&
          NextNoComment->isOneOf(TT_DesignatedInitializerPeriod,
                                 TT_DesignatedInitializerLSquare));
+    BreakBeforeParameter = EndsInComma;
     if (Current.ParameterCount > 1)
       NestedBlockIndent = std::max(NestedBlockIndent, State.Column + 1);
   } else {
     NewIndent = Style.ContinuationIndentWidth +
                 std::max(State.Stack.back().LastSpace,
                          State.Stack.back().StartOfFunctionCall);
 
     // Ensure that different different brackets force relative alignment, e.g.:

cfe/trunk/unittests/Format/FormatTest.cpp

 
 TEST_F(FormatTest, LayoutBraceInitializersInReturnStatement) {
   verifyFormat("return (a)(b){1, 2, 3};");
 }
 
 TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
   verifyFormat("vector<int> x{1, 2, 3, 4};");
   verifyFormat("vector<int> x{\n"
-               "    1, 2, 3, 4,\n"
+               "    1,\n"
+               "    2,\n"
+               "    3,\n"
+               "    4,\n"
                "};");
   verifyFormat("vector<T> x{{}, {}, {}, {}};");
   verifyFormat("f({1, 2});");
   verifyFormat("auto v = Foo{-1};");
   verifyFormat("f({1, 2}, {{2, 3}, {4, 5}}, c, {d});");
   verifyFormat("Class::Class : member{1, 2, 3} {}");
   verifyFormat("new vector<int>{1, 2, 3};");
   verifyFormat("new int[3]{1, 2, 3};");
verifyFormat("class C : public D {\n"
                "};");
   verifyFormat("class C : public A {\n"
                "  class D : public B {\n"
                "    void f() { int i{2}; }\n"
                "  };\n"
                "};");
   verifyFormat("#define A {a, a},");
 
+  // Binpacking only if there is no trailing comma
+  verifyFormat("const Aaaaaa aaaaa = {aaaaaaaaaa, bbbbbbbbbb,\n"
+               "                      cccccccccc, dddddddddd};",
+         getLLVMStyleWithColumns(50));
+  verifyFormat("const Aaaaaa aaaaa = {\n"
+               "    aaaaaaaaaaa,\n"
+               "    bbbbbbbbbbb,\n"
+               "    ccccccccccc,\n"
+               "    ddddddddddd,\n"
+               "};", getLLVMStyleWithColumns(50));
+
   // Cases where distinguising braced lists and blocks is hard.
   verifyFormat("vector<int> v{12} GUARDED_BY(mutex);");
   verifyFormat("void f() {\n"
                "  return; // comment\n"
                "}\n"
                "SomeType t;");
   verifyFormat("void f() {\n"
                "  if (a) {\n"
EXPECT_EQ("std::vector<int> MyNumbers{\n"
             "    1,\n"
             "    // Second element:\n"
             "    2};",
             format("std::vector<int> MyNumbers{// First element:\n"
                    "                           1,\n"
                    "                           // Second element:\n"
                    "                           2};",
                    getLLVMStyleWithColumns(30)));
-  // A trailing comma should still lead to an enforced line break.
+  // A trailing comma should still lead to an enforced line break and no
+  // binpacking.
   EXPECT_EQ("vector<int> SomeVector = {\n"
             "    // aaa\n"
-            "    1, 2,\n"
+            "    1,\n"
+            "    2,\n"
             "};",
             format("vector<int> SomeVector = { // aaa\n"
                    "    1, 2, };"));
 
   FormatStyle ExtraSpaces = getLLVMStyle();
   ExtraSpaces.Cpp11BracedListStyle = false;
   ExtraSpaces.ColumnLimit = 75;
   verifyFormat("vector<int> x{ 1, 2, 3, 4 };", ExtraSpaces);
verifyFormat(
       "     aaaaaaaaaaaaa, aaaaaaa, aaa},\n"
       "    {aaa, aaa},\n"
       "    {aaa, aaa},\n"
       "    {aaaa, aaaa, aaaa, aaaa, aaaa, aaaa, aaaa, aaa},\n"
       "    {aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaa,\n"
       "     aaaaaaaaaaaa, a, aaaaaaaaaa, aaaaaaaaa, aaa}};");
 
   // No column layout should be used here.
-  verifyFormat("aaaaaaaaaaaaaaa = {aaaaaaaaaaaaaaaaaaaaaaaaaaa, 0, 0,\n"
+  verifyFormat("aaaaaaaaaaaaaaa = {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, 0, 0,\n"
                "                   bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb};");
 
   verifyNoCrash("a<,");
 
   // No braced initializer here.
   verifyFormat("void f() {\n"
                "  struct Dummy {};\n"
                "  f(v);\n"

cfe/trunk/unittests/Format/FormatTestJava.cpp

verifyFormat("public enum VeryLongEnum {\n"
                "  SECOND_ENUM(\"a\", \"b\", \"c\");\n"
                "  private VeryLongEnum(String a, String b, String c) {}\n"
                "}\n");
 }
 
 TEST_F(FormatTestJava, ArrayInitializers) {
   verifyFormat("new int[] {1, 2, 3, 4};");
   verifyFormat("new int[] {\n"
-               "    1, 2, 3, 4,\n"
+               "    1,\n"
+               "    2,\n"
+               "    3,\n"
+               "    4,\n"
                "};");
 
   FormatStyle Style = getStyleWithColumns(65);
   Style.Cpp11BracedListStyle = false;
   verifyFormat(
       "expected = new int[] { 10, 10, 10, 10, 10, 10, 10, 10, 10, 10,\n"
       "  100, 100, 100, 100, 100, 100, 100, 100, 100, 100 };",
       Style);