[cfe-dev] clang-format: inconsistency in function arg layout (C++)

[Oleg Smolsky via cfe-dev]

OK, the functionality is a special case introduced in 2014 here: 
8228889b01404d7e59270b1f97a83977531a7748.

The minimal hack is to check for the preceding comma... but that breaks 
some "literal" cases... So, I need to make these selections even more 
particular (to either exclude literals or only include lambdas). Does 
this sound right?

On 2018-09-10 12:47, Oleg Smolsky wrote:
> Hi, I've just boiled down an interesting C++ lambda formatting trait 
> and would like to clarify the tool's behavior. Consider the following 
> snippet (please view with a fixed-width font):
>
> void f() {
>   something->One(
>       [this] {
>         Do1();
>         Do2();
>       },
>       1);
>   something->Two(1,
>                  [this] {
>                    Do1();
>                    Do2();
>                  },
>                  1);
> }
>
> There is an inconsistency in the way lambda args are formatted, 
> depending on whether it is first (the "One()" call above) or not (the 
> "Two()" call above). Is there some internal guide that the tool uses 
> to decide between the two layouts? Or is it just an artifact of the 
> implementation?
>
> More generally, would you entertain a patch that forces the format one 
> way or another? (Perhaps even with a user-defined setting?)
>
> Thanks,
> Oleg.
>
-------------- next part --------------
From 317dbf753d276e4204529d484401c9b1e2cf8514 Mon Sep 17 00:00:00 2001
From: Oleg Smolsky <oleg at cohesity.com>
Date: Mon, 10 Sep 2018 13:39:14 -0700
Subject: [PATCH] lib/Format: tweaked another case of lambda formatting

---
 lib/Format/ContinuationIndenter.cpp |  4 ++--
 unittests/Format/FormatTest.cpp     | 10 ++++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index f035aa71e5..a69b1226f8 100644
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -303,7 +303,7 @@ bool ContinuationIndenter::canBreak(const LineState &State) {
 
   // Don't create a 'hanging' indent if there are multiple blocks in a single
   // statement.
-  if (Previous.is(tok::l_brace) && State.Stack.size() > 1 &&
+  if (Previous.isOneOf(tok::l_brace, tok::comma) && State.Stack.size() > 1 &&
       State.Stack[State.Stack.size() - 2].NestedBlockInlined &&
       State.Stack[State.Stack.size() - 2].HasMultipleNestedBlocks)
     return false;
@@ -1145,7 +1145,7 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
       !Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
     State.Stack.back().NestedBlockInlined =
         !Newline &&
-        (Previous->isNot(tok::l_paren) || Previous->ParameterCount > 1);
+        (!Previous->isOneOf(tok::l_paren, tok::comma) || Previous->ParameterCount > 1);
   }
 
   moveStatePastFakeLParens(State, Newline);
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index e7c467547a..48c6c0bf33 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -11691,6 +11691,16 @@ TEST_F(FormatTest, FormatsLambdas) {
                "      return j;\n"
                "    });");
 
+  verifyFormat("void f() {\n"
+               "  something->One(\n"
+               "      1,\n"
+               "      [this] {\n"
+               "        Do1();\n"
+               "        Do2();\n"
+               "      },\n"
+               "      1);\n"
+               "}\n");
+
   // More complex introducers.
   verifyFormat("return [i, args...] {};");
 
-- 
2.15.1