r189466 - clang-format: Improve token breaking behavior.

[Daniel Jasper]

Author: djasper
Date: Wed Aug 28 05:03:58 2013
New Revision: 189466

URL: http://llvm.org/viewvc/llvm-project?rev=189466&view=rev
Log:
clang-format: Improve token breaking behavior.

Two changes:
* Don't add an extra penalty on breaking the same token multiple times.
  Generally, we should prefer not to break, but once we break, the
  normal line breaking penalties apply.
* Slightly increase the penalty for breaking comments. In general, the
  author has put some thought into how to break the comment and we
  should not overwrite this unnecessarily.

With a 40-column column limit, formatting
  aaaaaa("aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa");

Leads to:
Before:
  aaaaaa(
      "aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa "
      "aaaaaaaaaaaaaaaa");

After:
  aaaaaa("aaaaaaaaaaaaaaaa "
         "aaaaaaaaaaaaaaaa "
         "aaaaaaaaaaaaaaaa");

Modified:
    cfe/trunk/lib/Format/ContinuationIndenter.cpp
    cfe/trunk/lib/Format/Format.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=189466&r1=189465&r2=189466&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Wed Aug 28 05:03:58 2013
@@ -666,8 +666,7 @@ unsigned ContinuationIndenter::breakProt
       assert(NewRemainingTokenColumns < RemainingTokenColumns);
       if (!DryRun)
         Token->insertBreak(LineIndex, TailOffset, Split, Whitespaces);
-      Penalty += Current.is(tok::string_literal) ? Style.PenaltyBreakString
-                                                 : Style.PenaltyBreakComment;
+      Penalty += Current.SplitPenalty;
       unsigned ColumnsUsed =
           Token->getLineLengthAfterSplit(LineIndex, TailOffset, Split.first);
       if (ColumnsUsed > getColumnLimit()) {
@@ -691,6 +690,9 @@ unsigned ContinuationIndenter::breakProt
         State.Stack[i].BreakBeforeParameter = true;
     }
 
+    Penalty += Current.is(tok::string_literal) ? Style.PenaltyBreakString
+                                               : Style.PenaltyBreakComment;
+
     State.Stack.back().LastSpace = StartColumn;
   }
   return Penalty;

Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=189466&r1=189465&r2=189466&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Aug 28 05:03:58 2013
@@ -152,7 +152,7 @@ namespace clang {
 namespace format {
 
 void setDefaultPenalties(FormatStyle &Style) {
-  Style.PenaltyBreakComment = 45;
+  Style.PenaltyBreakComment = 60;
   Style.PenaltyBreakFirstLessLess = 120;
   Style.PenaltyBreakString = 1000;
   Style.PenaltyExcessCharacter = 1000000;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=189466&r1=189465&r2=189466&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Aug 28 05:03:58 2013
@@ -970,8 +970,8 @@ TEST_F(FormatTest, DontSplitLineComments
 }
 
 TEST_F(FormatTest, PriorityOfCommentBreaking) {
-  EXPECT_EQ("if (xxx == yyy && // aaaaaaaaaaaa\n"
-            "                  // bbbbbbbbb\n"
+  EXPECT_EQ("if (xxx ==\n"
+            "        yyy && // aaaaaaaaaaaa bbbbbbbbb\n"
             "    zzz)\n"
             "  q();",
             format("if (xxx == yyy && // aaaaaaaaaaaa bbbbbbbbb\n"
@@ -5392,10 +5392,10 @@ TEST_F(FormatTest, BreakStringLiteralsBe
             "    \"f\");",
             format("someFunction1234567890(\"aaabbbcccdddeeefff\");",
                    getLLVMStyleWithColumns(24)));
-  EXPECT_EQ("someFunction(\n"
-            "    \"aaabbbcc \"\n"
-            "    \"dddeeefff\");",
-            format("someFunction(\"aaabbbcc dddeeefff\");",
+  EXPECT_EQ("someFunction(\"aaabbbcc \"\n"
+            "             \"ddde \"\n"
+            "             \"efff\");",
+            format("someFunction(\"aaabbbcc ddde efff\");",
                    getLLVMStyleWithColumns(25)));
   EXPECT_EQ("someFunction(\"aaabbbccc \"\n"
             "             \"ddeeefff\");",
@@ -5413,6 +5413,12 @@ TEST_F(FormatTest, BreakStringLiteralsBe
             "  int i;",
             format("#define A string s = \"1234567890\"; int i;",
                    getLLVMStyleWithColumns(20)));
+  // FIXME: Put additional penalties on breaking at non-whitespace locations.
+  EXPECT_EQ("someFunction(\"aaabbbcc \"\n"
+            "             \"dddeeeff\"\n"
+            "             \"f\");",
+            format("someFunction(\"aaabbbcc dddeeefff\");",
+                   getLLVMStyleWithColumns(25)));
 }
 
 TEST_F(FormatTest, DoNotBreakStringLiteralsInEscapeSequence) {