r184076 - Fixes incorrect indentation of line comments after break and re-alignment.

[Alexander Kornienko]

Author: alexfh
Date: Mon Jun 17 07:59:44 2013
New Revision: 184076

URL: http://llvm.org/viewvc/llvm-project?rev=184076&view=rev
Log:
Fixes incorrect indentation of line comments after break and re-alignment.

Summary:
Selectively propagate the information about token kind in
WhitespaceManager::replaceWhitespaceInToken.For correct alignment of new
segments of line comments in order to align them correctly. Don't set
BreakBeforeParameter in breakProtrudingToken for line comments, as it introduces
a break after the _next_ parameter. Added tests for related functions.

Reviewers: klimek

Reviewed By: klimek

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D980

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

Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=184076&r1=184075&r2=184076&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Mon Jun 17 07:59:44 2013
@@ -873,8 +873,14 @@ private:
 
     if (BreakInserted) {
       State.Column = PositionAfterLastLineInToken;
-      for (unsigned i = 0, e = State.Stack.size(); i != e; ++i)
-        State.Stack[i].BreakBeforeParameter = true;
+      // If we break the token inside a parameter list, we need to break before
+      // the next parameter on all levels, so that the next parameter is clearly
+      // visible. Line comments already introduce a break.
+      if (Current.Type != TT_LineComment) {
+        for (unsigned i = 0, e = State.Stack.size(); i != e; ++i)
+          State.Stack[i].BreakBeforeParameter = true;
+      }
+
       State.Stack.back().LastSpace = StartColumn;
     }
     return Penalty;

Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=184076&r1=184075&r2=184076&view=diff
==============================================================================
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Mon Jun 17 07:59:44 2013
@@ -65,10 +65,13 @@ void WhitespaceManager::replaceWhitespac
                         Tok.getStartOfNonWhitespace().getLocWithOffset(
                             Offset + ReplaceChars)),
       Spaces, Spaces, Newlines, PreviousPostfix, CurrentPrefix,
-      // FIXME: Unify token adjustment, so we don't split it between
-      // BreakableToken and the WhitespaceManager. That would also allow us to
-      // correctly store a tok::TokenKind instead of rolling our own enum.
-      tok::unknown, InPPDirective && !Tok.IsFirst));
+      // If we don't add a newline this change doesn't start a comment. Thus,
+      // when we align line comments, we don't need to treat this change as one.
+      // FIXME: We still need to take this change in account to properly
+      // calculate the new length of the comment and to calculate the changes
+      // for which to do the alignment when aligning comments.
+      Tok.Type == TT_LineComment && Newlines > 0 ? tok::comment : tok::unknown,
+      InPPDirective && !Tok.IsFirst));
 }
 
 const tooling::Replacements &WhitespaceManager::generateReplacements() {

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=184076&r1=184075&r2=184076&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jun 17 07:59:44 2013
@@ -876,6 +876,16 @@ TEST_F(FormatTest, SplitsLongCxxComments
             format("// A comment before a macro definition\n"
                    "#define a b",
                    getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("void ffffff(int aaaaaaaaa,  // wwww\n"
+            "            int a, int bbb, // xxxxxxx\n"
+            "                            // yyyyyyyyy\n"
+            "            int c, int d, int e) {}",
+            format("void ffffff(\n"
+                   "    int aaaaaaaaa, // wwww\n"
+                   "    int a,\n"
+                   "    int bbb, // xxxxxxx yyyyyyyyy\n"
+                   "    int c, int d, int e) {}",
+                   getLLVMStyleWithColumns(40)));
 }
 
 TEST_F(FormatTest, PriorityOfCommentBreaking) {
@@ -2456,6 +2466,14 @@ TEST_F(FormatTest, BreaksDesireably) {
                "          Line.Tokens[i - 1].Tok.isNot(tok::l_paren) &&\n"
                "          Line.Tokens[i - 1].Tok.isNot(tok::l_square);\n"
                "    }\n  }\n}");
+
+  // Break on an outer level if there was a break on an inner level.
+  EXPECT_EQ("f(g(h(a, // comment\n"
+            "      b, c),\n"
+            "    d, e),\n"
+            "  x, y);",
+            format("f(g(h(a, // comment\n"
+                   "    b, c), d, e), x, y);"));
 }
 
 TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) {
@@ -4565,6 +4583,18 @@ TEST_F(FormatTest, BreakStringLiterals)
             format("variable = f(\"long string literal\", short, "
                    "loooooooooooooooooooong);",
                    getLLVMStyleWithColumns(20)));
+
+  EXPECT_EQ("f(g(\"long string \"\n"
+            "    \"literal\"),\n"
+            "  b);",
+            format("f(g(\"long string literal\"), b);",
+                   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("f(g(\"long string \"\n"
+            "    \"literal\",\n"
+            "    a),\n"
+            "  b);",
+            format("f(g(\"long string literal\", a), b);",
+                   getLLVMStyleWithColumns(20)));
   EXPECT_EQ(
       "f(\"one two\".split(\n"
       "    variable));",