[clang-format] tweaked another case of lambda formatting

Summary:
This is done in order to improve cases where the lambda's body is moved too far to the right. Consider the following snippet with column limit set to 79:

```
void f() {
  leader::MakeThisCallHere(&leader_service_,
                           cq_.get(),
                           [this, liveness](const leader::ReadRecordReq& req,
                                            std::function<void()> done) {
                             logger_->HandleReadRecord(
                                 req, resp, std::move(done));
                           });

  leader::MakeAnother(&leader_service_,
                      cq_.get(),
                      [this, liveness](const leader::ReadRecordReq& req,
                                       std::function<void()> done) {
                        logger_->HandleReadRecord(
                            req, resp, std::move(done), a);
                      });
}
```

The tool favors extra indentation for the lambda body and so the code incurs extra wrapping and adjacent calls are indented to a different level. I find this behavior annoying and I'd like the tool to favor new lines and, thus, use the extra width.

The fix, reduced, brings the following formatting.

Before:

    function(1,
             [] {
               DoStuff();
               //
             },
             1);

After:

    function(
        1,
        [] {
          DoStuff();
          //
        },
        1);

Refer to the new tests in FormatTest.cpp

Contributed by oleg.smolsky!

Reviewers: djasper, klimek, krasimir

Subscribers: cfe-commits, owenpan

Tags: #clang

Differential Revision: https://reviews.llvm.org/D52676

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

diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index 89d346d8a29ce7f0bc67c8c1a30759eea6403562..c369b94b99878590a13aa14ad9f80184fef756bd 100644 (file)
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -1135,7 +1135,8 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
   //   }, a, b, c);
   if (Current.isNot(tok::comment) && Previous &&
       Previous->isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) &&
-      !Previous->is(TT_DictLiteral) && State.Stack.size() > 1) {
+      !Previous->is(TT_DictLiteral) && State.Stack.size() > 1 &&
+      !State.Stack.back().HasMultipleNestedBlocks) {
     if (State.Stack[State.Stack.size() - 2].NestedBlockInlined && Newline)
       for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i)
         State.Stack[i].NoLineBreak = true;

diff --git a/lib/Format/FormatToken.h b/lib/Format/FormatToken.h
index 6e801a346b82da0bb5ad4705c276e4d335695705..10390c42911b87ddd8109c79e4e8d8bd9826ed1d 100644 (file)
--- a/lib/Format/FormatToken.h
+++ b/lib/Format/FormatToken.h
@@ -599,6 +599,8 @@ public:
   /// Notifies the \c Role that a comma was found.
   virtual void CommaFound(const FormatToken *Token) {}
 
+  virtual const FormatToken *lastComma() { return nullptr; }
+
 protected:
   const FormatStyle &Style;
 };
@@ -621,6 +623,12 @@ public:
     Commas.push_back(Token);
   }
 
+  const FormatToken *lastComma() override {
+    if (Commas.empty())
+      return nullptr;
+    return Commas.back();
+  }
+
 private:
   /// A struct that holds information on how to format a given list with
   /// a specific number of columns.

diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index d032fd7cb95bfb52d903f7bdb4e2708bbb116095..6f9b71b4b1ec4ec3bc715a2c1b98f33eed35ca99 100644 (file)
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -3049,6 +3049,30 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
       return true;
   }
 
+  // Deal with lambda arguments in C++ - we want consistent line breaks whether
+  // they happen to be at arg0, arg1 or argN. The selection is a bit nuanced
+  // as aggressive line breaks are placed when the lambda is not the last arg.
+  if ((Style.Language == FormatStyle::LK_Cpp ||
+       Style.Language == FormatStyle::LK_ObjC) &&
+      Left.is(tok::l_paren) && Left.BlockParameterCount > 0 &&
+      !Right.isOneOf(tok::l_paren, TT_LambdaLSquare)) {
+    // Multiple lambdas in the same function call force line breaks.
+    if (Left.BlockParameterCount > 1)
+      return true;
+
+    // A lambda followed by another arg forces a line break.
+    if (!Left.Role)
+      return false;
+    auto Comma = Left.Role->lastComma();
+    if (!Comma)
+      return false;
+    auto Next = Comma->getNextNonComment();
+    if (!Next)
+      return false;
+    if (!Next->isOneOf(TT_LambdaLSquare, tok::l_brace, tok::caret))
+      return true;
+  }
+
   return false;
 }
 

diff --git a/lib/Format/UnwrappedLineFormatter.cpp b/lib/Format/UnwrappedLineFormatter.cpp
index 76b495f453febc1d42b25f42487802f809af8bde..6b6a9aff461ab6c4875e4514e0f29871897c781e 100644 (file)
--- a/lib/Format/UnwrappedLineFormatter.cpp
+++ b/lib/Format/UnwrappedLineFormatter.cpp
@@ -988,8 +988,7 @@ private:
       Path.push_front(Best);
       Best = Best->Previous;
     }
-    for (std::deque<StateNode *>::iterator I = Path.begin(), E = Path.end();
-         I != E; ++I) {
+    for (auto I = Path.begin(), E = Path.end(); I != E; ++I) {
       unsigned Penalty = 0;
       formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
       Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
@@ -998,8 +997,8 @@ private:
         printLineState((*I)->Previous->State);
         if ((*I)->NewLine) {
           llvm::dbgs() << "Penalty for placing "
-                       << (*I)->Previous->State.NextToken->Tok.getName() << ": "
-                       << Penalty << "\n";
+                       << (*I)->Previous->State.NextToken->Tok.getName()
+                       << " on a new line: " << Penalty << "\n";
         }
       });
     }

diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 86b1ca2f17deebf5760812c248541c61406df636..635a34499da6a455cd2fbe621c37b9b776d05bde 100644 (file)
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -3277,11 +3277,12 @@ TEST_F(FormatTest, LayoutNestedBlocks) {
                      "});");
   FormatStyle Style = getGoogleStyle();
   Style.ColumnLimit = 45;
-  verifyFormat("Debug(aaaaa,\n"
-               "      {\n"
-               "        if (aaaaaaaaaaaaaaaaaaaaaaaa) return;\n"
-               "      },\n"
-               "      a);",
+  verifyFormat("Debug(\n"
+               "    aaaaa,\n"
+               "    {\n"
+               "      if (aaaaaaaaaaaaaaaaaaaaaaaa) return;\n"
+               "    },\n"
+               "    a);",
                Style);
 
   verifyFormat("SomeFunction({MACRO({ return output; }), b});");
@@ -11739,6 +11740,18 @@ TEST_F(FormatTest, FormatsLambdas) {
                "        x.end(),   //\n"
                "        [&](int, int) { return 1; });\n"
                "}\n");
+  verifyFormat("void f() {\n"
+               "  other.other.other.other.other(\n"
+               "      x.begin(), x.end(),\n"
+               "      [something, rather](int, int, int, int, int, int, int) { return 1; });\n"
+               "}\n");
+  verifyFormat("void f() {\n"
+               "  other.other.other.other.other(\n"
+               "      x.begin(), x.end(),\n"
+               "      [something, rather](int, int, int, int, int, int, int) {\n"
+               "        //\n"
+               "      });\n"
+               "}\n");
   verifyFormat("SomeFunction([]() { // A cool function...\n"
                "  return 43;\n"
                "});");
@@ -11790,9 +11803,9 @@ TEST_F(FormatTest, FormatsLambdas) {
   verifyFormat("SomeFunction({[&] {\n"
                "  // comment\n"
                "}});");
-  verifyFormat("virtual aaaaaaaaaaaaaaaa(std::function<bool()> bbbbbbbbbbbb =\n"
-               "                             [&]() { return true; },\n"
-               "                         aaaaa aaaaaaaaa);");
+  verifyFormat("virtual aaaaaaaaaaaaaaaa(\n"
+               "    std::function<bool()> bbbbbbbbbbbb = [&]() { return true; },\n"
+               "    aaaaa aaaaaaaaa);");
 
   // Lambdas with return types.
   verifyFormat("int c = []() -> int { return 2; }();\n");
@@ -11819,17 +11832,76 @@ TEST_F(FormatTest, FormatsLambdas) {
                "  return 1; //\n"
                "};");
 
-  // Multiple lambdas in the same parentheses change indentation rules.
+  // Multiple lambdas in the same parentheses change indentation rules. These
+  // lambdas are forced to start on new lines.
   verifyFormat("SomeFunction(\n"
                "    []() {\n"
-               "      int i = 42;\n"
-               "      return i;\n"
+               "      //\n"
                "    },\n"
                "    []() {\n"
-               "      int j = 43;\n"
-               "      return j;\n"
+               "      //\n"
                "    });");
 
+  // A lambda passed as arg0 is always pushed to the next line.
+  verifyFormat("SomeFunction(\n"
+               "    [this] {\n"
+               "      //\n"
+               "    },\n"
+               "    1);\n");
+
+  // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like the arg0
+  // case above.
+  auto Style = getGoogleStyle();
+  Style.BinPackArguments = false;
+  verifyFormat("SomeFunction(\n"
+               "    a,\n"
+               "    [this] {\n"
+               "      //\n"
+               "    },\n"
+               "    b);\n",
+               Style);
+  verifyFormat("SomeFunction(\n"
+               "    a,\n"
+               "    [this] {\n"
+               "      //\n"
+               "    },\n"
+               "    b);\n");
+
+  // A lambda with a very long line forces arg0 to be pushed out irrespective of
+  // the BinPackArguments value (as long as the code is wide enough).
+  verifyFormat("something->SomeFunction(\n"
+               "    a,\n"
+               "    [this] {\n"
+               "      D0000000000000000000000000000000000000000000000000000000000001();\n"
+               "    },\n"
+               "    b);\n");
+
+  // A multi-line lambda is pulled up as long as the introducer fits on the previous
+  // line and there are no further args.
+  verifyFormat("function(1, [this, that] {\n"
+               "  //\n"
+               "});\n");
+  verifyFormat("function([this, that] {\n"
+               "  //\n"
+               "});\n");
+  // FIXME: this format is not ideal and we should consider forcing the first arg
+  // onto its own line.
+  verifyFormat("function(a, b, c, //\n"
+               "         d, [this, that] {\n"
+               "           //\n"
+               "         });\n");
+
+  // Multiple lambdas are treated correctly even when there is a short arg0.
+  verifyFormat("SomeFunction(\n"
+               "    1,\n"
+               "    [this] {\n"
+               "      //\n"
+               "    },\n"
+               "    [this] {\n"
+               "      //\n"
+               "    },\n"
+               "    1);\n");
+
   // More complex introducers.
   verifyFormat("return [i, args...] {};");