r193345 - clang-format: Be more conservative about column layout formatting.

[Daniel Jasper]

Author: djasper
Date: Thu Oct 24 09:14:49 2013
New Revision: 193345

URL: http://llvm.org/viewvc/llvm-project?rev=193345&view=rev
Log:
clang-format: Be more conservative about column layout formatting.

Specifically, if a braced list has at least one nested braced list,
format it either all on one line or in one column (i.e. one item per
line).

This seems in general to be an improvement as the structure of nested
braced lists can make a tightly packed outer braced list hard to read.

Modified:
    cfe/trunk/lib/Format/FormatToken.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/FormatToken.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.cpp?rev=193345&r1=193344&r2=193345&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.cpp (original)
+++ cfe/trunk/lib/Format/FormatToken.cpp Thu Oct 24 09:14:49 2013
@@ -99,12 +99,15 @@ void CommaSeparatedList::precomputeForma
   // trailing comments which are otherwise ignored for column alignment.
   SmallVector<unsigned, 8> EndOfLineItemLength;
 
+  bool HasNestedBracedList = false;
   for (unsigned i = 0, e = Commas.size() + 1; i != e; ++i) {
     // Skip comments on their own line.
     while (ItemBegin->HasUnescapedNewline && ItemBegin->isTrailingComment())
       ItemBegin = ItemBegin->Next;
 
     MustBreakBeforeItem.push_back(ItemBegin->MustBreakBefore);
+    if (ItemBegin->is(tok::l_brace))
+      HasNestedBracedList = true;
     const FormatToken *ItemEnd = NULL;
     if (i == Commas.size()) {
       ItemEnd = Token->MatchingParen;
@@ -142,7 +145,7 @@ void CommaSeparatedList::precomputeForma
     ColumnFormat Format;
     Format.Columns = Columns;
     Format.ColumnSizes.resize(Columns);
-    Format.LineCount = 0;
+    Format.LineCount = 1;
     bool HasRowWithSufficientColumns = false;
     unsigned Column = 0;
     for (unsigned i = 0, e = ItemLengths.size(); i != e; ++i) {
@@ -172,6 +175,11 @@ void CommaSeparatedList::precomputeForma
     if (Format.TotalWidth > Style.ColumnLimit)
       continue;
 
+    // If this braced list has nested braced list, we format it either with one
+    // element per line or with all elements on one line.
+    if (HasNestedBracedList && Columns > 1 && Format.LineCount > 1)
+      continue;
+
     Formats.push_back(Format);
   }
 }

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=193345&r1=193344&r2=193345&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Oct 24 09:14:49 2013
@@ -1864,14 +1864,14 @@ TEST_F(FormatTest, NestedStaticInitializ
       "      333333333333333333333333333333}},\n"
       "    {{1, 2, 3}}, {{1, 2, 3}}};");
 
-  // FIXME: We might at some point want to handle this similar to parameter
-  // lists, where we have an option to put each on a single line.
   verifyFormat(
       "struct {\n"
       "  unsigned bit;\n"
       "  const char *const name;\n"
-      "} kBitsToOs[] = { { kOsMac, \"Mac\" },     { kOsWin, \"Windows\" },\n"
-      "                  { kOsLinux, \"Linux\" }, { kOsCrOS, \"Chrome OS\" } };");
+      "} kBitsToOs[] = { { kOsMac, \"Mac\" },\n"
+      "                  { kOsWin, \"Windows\" },\n"
+      "                  { kOsLinux, \"Linux\" },\n"
+      "                  { kOsCrOS, \"Chrome OS\" } };");
 }
 
 TEST_F(FormatTest, FormatsSmallMacroDefinitionsInSingleLine) {
@@ -4539,8 +4539,10 @@ TEST_F(FormatTest, FormatsBracedListsinC
                "                  1, 1, 1, 1,\n"
                "                  /**/ /**/ };",
                getLLVMStyleWithColumns(39));
-  verifyFormat("return { { aaaaaaaaaaaaaaaaaaaaa }, { aaaaaaaaaaaaaaaaaaa },\n"
-               "         { aaaaaaaaaaaaaaaaaaaaa }, { aaaaaaaaaaaaaaaaa } };",
+  verifyFormat("return { { aaaaaaaaaaaaaaaaaaaaa },\n"
+               "         { aaaaaaaaaaaaaaaaaaa },\n"
+               "         { aaaaaaaaaaaaaaaaaaaaa },\n"
+               "         { aaaaaaaaaaaaaaaaa } };",
                getLLVMStyleWithColumns(60));
 }