r189211 - clang-format: Fix bug in column-layout formatting.

[Daniel Jasper]

Author: djasper
Date: Mon Aug 26 03:10:17 2013
New Revision: 189211

URL: http://llvm.org/viewvc/llvm-project?rev=189211&view=rev
Log:
clang-format: Fix bug in column-layout formatting.

Specific arrangements of comments after trailing commas could confuse
the column width calculation, e.g. in:

vector<int> x = { a, b,
                  /* some */ /* comment */ };

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=189211&r1=189210&r2=189211&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.cpp (original)
+++ cfe/trunk/lib/Format/FormatToken.cpp Mon Aug 26 03:10:17 2013
@@ -77,11 +77,13 @@ unsigned CommaSeparatedList::format(Line
 // assuming that the entire sequence is put on a single line.
 static unsigned CodePointsBetween(const FormatToken *Begin,
                                   const FormatToken *End) {
+  assert(End->TotalLength >= Begin->TotalLength);
   return End->TotalLength - Begin->TotalLength + Begin->CodePointCount;
 }
 
 void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) {
-  if (!Token->MatchingParen)
+  // FIXME: At some point we might want to do this for other lists, too.
+  if (!Token->MatchingParen || Token->isNot(tok::l_brace))
     return;
 
   FormatToken *ItemBegin = Token->Next;
@@ -92,11 +94,6 @@ void CommaSeparatedList::precomputeForma
   SmallVector<unsigned, 8> EndOfLineItemLength;
 
   for (unsigned i = 0, e = Commas.size() + 1; i != e; ++i) {
-    // If there is a trailing comma in the list, the next item will start at the
-    // closing brace. Don't create an extra item for this.
-    if (ItemBegin == Token->MatchingParen)
-      break;
-
     // Skip comments on their own line.
     while (ItemBegin->HasUnescapedNewline && ItemBegin->isTrailingComment())
       ItemBegin = ItemBegin->Next;
@@ -119,14 +116,17 @@ void CommaSeparatedList::precomputeForma
     } else {
       ItemEnd = Commas[i];
       // The comma is counted as part of the item when calculating the length.
-      ItemLengths.push_back(ItemEnd->TotalLength - ItemBegin->TotalLength +
-                            ItemBegin->CodePointCount);
+      ItemLengths.push_back(CodePointsBetween(ItemBegin, ItemEnd));
       // Consume trailing comments so the are included in EndOfLineItemLength.
       if (ItemEnd->Next && !ItemEnd->Next->HasUnescapedNewline &&
           ItemEnd->Next->isTrailingComment())
         ItemEnd = ItemEnd->Next;
     }
     EndOfLineItemLength.push_back(CodePointsBetween(ItemBegin, ItemEnd));
+    // If there is a trailing comma in the list, the next item will start at the
+    // closing brace. Don't create an extra item for this.
+    if (ItemEnd->getNextNonComment() == Token->MatchingParen)
+      break;
     ItemBegin = ItemEnd->Next;
   }
 

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=189211&r1=189210&r2=189211&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Aug 26 03:10:17 2013
@@ -3530,7 +3530,7 @@ TEST_F(FormatTest, IndentsRelativeToUnar
                "    bbbbbb);");
 }
 
-TEST_F(FormatTest, UndestandsOverloadedOperators) {
+TEST_F(FormatTest, UnderstandsOverloadedOperators) {
   verifyFormat("bool operator<();");
   verifyFormat("bool operator>();");
   verifyFormat("bool operator=();");
@@ -4191,6 +4191,10 @@ TEST_F(FormatTest, FormatsBracedListsinC
                "  1, 1, 1, 1, 1, 1, 1, 1, //\n"
                "};",
                getLLVMStyleWithColumns(39));
+  verifyFormat("vector<int> x = { 1, 1, 1, 1,\n"
+               "                  1, 1, 1, 1,\n"
+               "                  /**/ /**/ };",
+               getLLVMStyleWithColumns(39));
 }
 
 TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {