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");

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@189466 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index 798e4f3aecdca89f25a0ad8b6e9b280c60be4c89..f4ae5cce01b78cc96899b4219bd55650e8422d9d 100644 (file)
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -666,8 +666,7 @@ unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
       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::breakProtrudingToken(const FormatToken &Current,
         State.Stack[i].BreakBeforeParameter = true;
     }
 
+    Penalty += Current.is(tok::string_literal) ? Style.PenaltyBreakString
+                                               : Style.PenaltyBreakComment;
+
     State.Stack.back().LastSpace = StartColumn;
   }
   return Penalty;

diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 1cd55d729737c2874bc00da98b3b441246608cad..3982ba6a77180c9209e9e094870642b0b4dd854e 100644 (file)
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -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;

diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 067a259b2ce6a3df35c1fe64c5ebfc797398cc77..122b367a223c3364c3d9d719a66cf9dc348d21d0 100644 (file)
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -970,8 +970,8 @@ TEST_F(FormatTest, DontSplitLineCommentsWithEscapedNewlines) {
 }
 
 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, BreakStringLiteralsBeforeUnbreakableTokenSequence) {
             "    \"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, BreakStringLiteralsBeforeUnbreakableTokenSequence) {
             "  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) {