r197725 - clang-format: Add special case for leading comments in braced lists.

Daniel Jasper djasper at google.com
Thu Dec 19 13:41:37 PST 2013


Author: djasper
Date: Thu Dec 19 15:41:37 2013
New Revision: 197725

URL: http://llvm.org/viewvc/llvm-project?rev=197725&view=rev
Log:
clang-format: Add special case for leading comments in braced lists.

A comment following the "{" of a braced list seems to almost always
refer to the first element of the list and thus should be aligned
to it.

Before (with Cpp11 braced list style):
  SomeFunction({  // Comment 1
                   "first entry",
                   // Comment 2
                   "second entry"});
After:
  SomeFunction({// Comment 1
                "first entry",
                // Comment 2
                "second entry"});

Modified:
    cfe/trunk/lib/Format/ContinuationIndenter.cpp
    cfe/trunk/lib/Format/TokenAnnotator.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=197725&r1=197724&r2=197725&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Dec 19 15:41:37 2013
@@ -268,7 +268,7 @@ void ContinuationIndenter::addTokenOnCur
   }
 
   if (Previous.opensScope() && Previous.Type != TT_ObjCMethodExpr &&
-      Current.Type != TT_LineComment)
+      (Current.Type != TT_LineComment || Previous.BlockKind == BK_BracedInit))
     State.Stack.back().Indent = State.Column + Spaces;
   if (State.Stack.back().AvoidBinPacking && startsNextParameter(Current, Style))
     State.Stack.back().NoLineBreak = true;
@@ -596,9 +596,11 @@ unsigned ContinuationIndenter::moveState
         NewIndent = State.Stack.back().LastSpace;
         if (Current.opensBlockTypeList(Style)) {
           NewIndent += Style.IndentWidth;
+          NewIndent = std::min(State.Column + 2, NewIndent);
           ++NewIndentLevel;
         } else {
           NewIndent += Style.ContinuationIndentWidth;
+          NewIndent = std::min(State.Column + 1, NewIndent);
         }
       }
       const FormatToken *NextNoComment = Current.getNextNonComment();

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=197725&r1=197724&r2=197725&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Dec 19 15:41:37 2013
@@ -1072,11 +1072,15 @@ void TokenAnnotator::calculateFormatting
   FormatToken *Current = Line.First->Next;
   bool InFunctionDecl = Line.MightBeFunctionDecl;
   while (Current != NULL) {
-    if (Current->Type == TT_LineComment)
-      Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
-    else if (Current->SpacesRequiredBefore == 0 &&
-             spaceRequiredBefore(Line, *Current))
+    if (Current->Type == TT_LineComment) {
+      if (Current->Previous->BlockKind == BK_BracedInit)
+        Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
+      else
+        Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
+    } else if (Current->SpacesRequiredBefore == 0 && 
+             spaceRequiredBefore(Line, *Current)) {
       Current->SpacesRequiredBefore = 1;
+    }
 
     Current->MustBreakBefore =
         Current->MustBreakBefore || mustBreakBefore(Line, *Current);
@@ -1398,7 +1402,8 @@ bool TokenAnnotator::spaceRequiredBefore
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
                                      const FormatToken &Right) {
   if (Right.is(tok::comment)) {
-    return Right.NewlinesBefore > 0;
+    return Right.Previous->BlockKind != BK_BracedInit &&
+           Right.NewlinesBefore > 0;
   } else if (Right.Previous->isTrailingComment() ||
              (Right.is(tok::string_literal) &&
               Right.Previous->is(tok::string_literal))) {
@@ -1445,7 +1450,10 @@ bool TokenAnnotator::canBreakBefore(cons
   if (Right.isTrailingComment())
     // We rely on MustBreakBefore being set correctly here as we should not
     // change the "binding" behavior of a comment.
-    return false;
+    // The first comment in a braced lists is always interpreted as belonging to
+    // the first list element. Otherwise, it should be placed outside of the
+    // list.
+    return Left.BlockKind == BK_BracedInit;
   if (Left.is(tok::question) && Right.is(tok::colon))
     return false;
   if (Right.Type == TT_ConditionalExpr || Right.is(tok::question))

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=197725&r1=197724&r2=197725&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Dec 19 15:41:37 2013
@@ -4771,12 +4771,12 @@ TEST_F(FormatTest, LayoutCxx11Constructo
     verifyFormat("DoSomethingWithVector({} /* No data */);");
     verifyFormat("DoSomethingWithVector({ {} /* No data */ }, { { 1, 2 } });");
     verifyFormat(
-        "someFunction(OtherParam, BracedList{\n"
-        "                           // comment 1 (Forcing interesting break)\n"
-        "                           param1, param2,\n"
-        "                           // comment 2\n"
-        "                           param3, param4\n"
-        "                         });");
+        "someFunction(OtherParam,\n"
+        "             BracedList{ // comment 1 (Forcing interesting break)\n"
+        "                         param1, param2,\n"
+        "                         // comment 2\n"
+        "                         param3, param4 });",
+        getLLVMStyleWithColumns(75));
     verifyFormat(
         "std::this_thread::sleep_for(\n"
         "    std::chrono::nanoseconds{ std::chrono::seconds{ 1 } } / 5);");
@@ -4808,11 +4808,39 @@ TEST_F(FormatTest, LayoutCxx11Constructo
                  "  T member = {arg1, arg2};\n"
                  "};",
                  NoSpaces);
+
+    // FIXME: The alignment of these trailing comments might be bad. Then again,
+    // this might be utterly useless in real code.
     verifyFormat("Constructor::Constructor()\n"
-                 "    : some_value{ //\n"
-                 "          aaaaaaa //\n"
+                 "    : some_value{        //\n"
+                 "                 aaaaaaa //\n"
                  "      } {}",
                  NoSpaces);
+
+    // In braced lists, the first comment is always assumed to belong to the
+    // first element. Thus, it can be moved to the next or previous line as
+    // appropriate.
+    EXPECT_EQ("function({// First element:\n"
+              "          1,\n"
+              "          // Second element:\n"
+              "          2});",
+              format("function({\n"
+                     "    // First element:\n"
+                     "    1,\n"
+                     "    // Second element:\n"
+                     "    2});",
+                     NoSpaces));
+    NoSpaces.ColumnLimit = 30;
+    EXPECT_EQ(
+  "std::vector<int> MyNumbers{\n"
+  "    // First element:\n"
+  "    1,\n"
+  "    // Second element:\n"
+  "    2};",
+  format("std::vector<int> MyNumbers{// First element:\n"
+  "                           1,\n"
+  "                           // Second element:\n"
+  "                           2};", NoSpaces));
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {





More information about the cfe-commits mailing list