Fix hacky way of preventing a certain type of line break.

In general, we like to avoid line breaks like:

  ...
  SomeParameter, OtherParameter).DoSomething(
  ...

as they tend to make code really hard to read (how would you even indent the
next line?). Previously we have implemented this in a hacky way, which has now
shown to lead to problems. This fixes a few weird looking formattings, such as:

Before:
aaaaa(
    aaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)
        .aaaaa(aaaaa),
    aaaaaaaaaaaaaaaaaaaaa);
After:
aaaaa(aaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
            aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).aaaaa(aaaaa),
      aaaaaaaaaaaaaaaaaaaaa);

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

diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 41932666293a2cc8671e1e629b62512ce6583e9d..dcb5f8a6edae2c8cef0ba8d071eb0dbe3d30f8a9 100644 (file)
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -257,6 +257,7 @@ public:
     State.ParenLevel = 0;
     State.StartOfStringLiteral = 0;
     State.StartOfLineLevel = State.ParenLevel;
+    State.LowestLevelOnLine = State.ParenLevel;
     State.IgnoreStackForComparison = false;
 
     // The first token has already been indented and thus consumed.
@@ -412,6 +413,9 @@ private:
     /// \brief The \c ParenLevel at the start of this line.
     unsigned StartOfLineLevel;
 
+    /// \brief The lowest \c ParenLevel on the current line.
+    unsigned LowestLevelOnLine;
+
     /// \brief The start column of the string literal, if we're in a string
     /// literal sequence, 0 otherwise.
     unsigned StartOfStringLiteral;
@@ -448,6 +452,8 @@ private:
         return ParenLevel < Other.ParenLevel;
       if (StartOfLineLevel != Other.StartOfLineLevel)
         return StartOfLineLevel < Other.StartOfLineLevel;
+      if (LowestLevelOnLine != Other.LowestLevelOnLine)
+        return LowestLevelOnLine < Other.LowestLevelOnLine;
       if (StartOfStringLiteral != Other.StartOfStringLiteral)
         return StartOfStringLiteral < Other.StartOfStringLiteral;
       if (IgnoreStackForComparison || Other.IgnoreStackForComparison)
@@ -556,6 +562,7 @@ private:
       if (Current.isOneOf(tok::arrow, tok::period))
         State.Stack.back().LastSpace += Current.FormatTok.TokenLength;
       State.StartOfLineLevel = State.ParenLevel;
+      State.LowestLevelOnLine = State.ParenLevel;
 
       // Any break on this level means that the parent level has been broken
       // and we need to avoid bin packing there.
@@ -752,6 +759,8 @@ private:
       State.Stack.pop_back();
       --State.ParenLevel;
     }
+    State.LowestLevelOnLine =
+        std::min(State.LowestLevelOnLine, State.ParenLevel);
 
     // Remove scopes created by fake parenthesis.
     for (unsigned i = 0, e = Current.FakeRParens; i != e; ++i) {
@@ -996,6 +1005,14 @@ private:
         Previous.Parent &&
         Previous.Parent->isOneOf(tok::l_brace, tok::l_paren, tok::comma))
       return false;
+    // This prevents breaks like:
+    //   ...
+    //   SomeParameter, OtherParameter).DoSomething(
+    //   ...
+    // As they hide "DoSomething" and are generally bad for readability.
+    if (Previous.opensScope() &&
+        State.LowestLevelOnLine < State.StartOfLineLevel)
+      return false;
     return !State.Stack.back().NoLineBreak;
   }
 
@@ -1034,16 +1051,6 @@ private:
          (Previous.ClosesTemplateDeclaration && State.ParenLevel == 0)))
       return true;
 
-    // This prevents breaks like:
-    //   ...
-    //   SomeParameter, OtherParameter).DoSomething(
-    //   ...
-    // As they hide "DoSomething" and generally bad for readability.
-    if (Current.isOneOf(tok::period, tok::arrow) &&
-        getRemainingLength(State) + State.Column > getColumnLimit() &&
-        State.ParenLevel < State.StartOfLineLevel)
-      return true;
-
     if (Current.Type == TT_StartOfName && Line.MightBeFunctionDecl &&
         State.Stack.back().BreakBeforeParameter && State.ParenLevel == 0)
       return true;

diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 8d56d7236efe901df87163396fa75e791bf3a1ec..132dbb5aa2149081867df889a3c9add52e3857b8 100644 (file)
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -2610,6 +2610,9 @@ TEST_F(FormatTest, WrapsAtFunctionCallsIfNecessary) {
                "    .WillRepeatedly(Return(SomeValue));");
   verifyFormat("SomeMap[std::pair(aaaaaaaaaaaa, bbbbbbbbbbbbbbb)]\n"
                "    .insert(ccccccccccccccccccccccc);");
+  verifyFormat("aaaaa(aaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
+               "            aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).aaaaa(aaaaa),\n"
+               "      aaaaaaaaaaaaaaaaaaaaa);");
   verifyFormat(
       "aaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
       "      aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"