[clang-format] Fix AlignConsecutiveAssignments breaking lambda formatting.

[curdeius]

Summary

Fixes https://github.com/llvm/llvm-project/issues/52772.

This patch fixes the formatting of the code:

auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b                     = g([] {
  return;
});

which should be left as is, but before this patch was formatted to:

auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b                     = g([] {
  return;
                    });

[MyDeveloperDay]

LGTM

[curdeius]

Actually, I've just run it on a larger codebase and found that this patch misbehaves. Need to rework and add faulty test cases.
Sorry for the noise.

[curdeius]

Fix cases with lambdas. Added a (still) failing test with a ternary operator and multi-line continuation.

[curdeius]

This is still a WIP, but I'd like to have your opinion on how to tackle this problem.
What I'd like to do basically is to avoid shifting right the body of the lambda but still align things inside the lambda body.
This patch, for the moment, basically disables alignment in the lambdas which is not what I want.
Also, this approach is ugly.

All these problems were introduced somewhere between release 12 and 13. Does anyone know which commit could provoke this?

clang/unittests/Format/FormatTest.cpp

16605

This test case still fails, but I'm inclined to keep it commented for the moment and fix it later.

[MyDeveloperDay]

This patch, for the moment, basically disables alignment in the lambdas which is not what I want.

I sort of feel that's ok for now.

This is still a WIP, but I'd like to have your opinion on how to tackle this problem.

What are we trying to achieve? what is the expected output if not this? From what I can tell what you've put in your lists it looks ok

auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b                     = g([] {
  return;
});

[curdeius]

In D115972#3283229, @MyDeveloperDay wrote:

This patch, for the moment, basically disables alignment in the lambdas which is not what I want.

I sort of feel that's ok for now.

Ok.

This is still a WIP, but I'd like to have your opinion on how to tackle this problem.

What are we trying to achieve? what is the expected output if not this? From what I can tell what you've put in your lists it looks ok

auto aaaaaaaaaaaaaaaaaaaaa = {};
auto b                     = g([] {
  return;
});

The expected output is ok indeed, I'm rather not sure about the way I solve it here.

But yeah, it's better than what we have now.

[curdeius]

• Add FIXME.

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

16608

What determines this indent? I can't see how it should be calculated.

[curdeius]

clang/unittests/Format/FormatTest.cpp

16608

I didn't think about it before. But now I see, it's computed as in:

verifyFormat("auto b = f(aaaaaaaaaaaaaaaaaaaaaaaaa,\n"
             "           ccc ? aaaaa : bbbbb,\n"
             "           dddddddddddddddddddddddddd);",
             Alignment);

i.e. as if there were no alignment at = sign. So it's "aligned" to the previous position just after of f(.

Maybe we should align it the same way we do when function arguments don't fit into the line?
Note, I opened https://github.com/llvm/llvm-project/issues/53497 for it.

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

16608

Maybe we should align it the same way we do when function arguments don't fit into the line?

Yes we should. (Next line and add ContinuationIndent, should it be, right?)

But yes, that could be handled in a different commit.

clang/lib/Format/WhitespaceManager.cpp

for (unsigned i = Start; i != End; ++i) {
     // as mentioned in the ScopeStack comment.
     if (InsideNestedScope && Changes[i].NewlinesBefore > 0) {
       unsigned ScopeStart = ScopeStack.back();
       auto ShouldShiftBeAdded = [&] {
         // Function declaration
         if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName))
           return true;
 
+        // Lambda.
+        if (Changes[ScopeStart - 1].Tok->is(TT_LambdaLBrace))
+          return false;
+
         // Continued function declaration
         if (ScopeStart > Start + 1 &&
             Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName))
           return true;
 
         // Continued function call
         if (ScopeStart > Start + 1 &&
             Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
-            Changes[ScopeStart - 1].Tok->is(tok::l_paren))
+            Changes[ScopeStart - 1].Tok->is(tok::l_paren) &&
+            Changes[ScopeStart].Tok->isNot(TT_LambdaLSquare)) {
+          if (Changes[i].Tok->MatchingParen &&
+              Changes[i].Tok->MatchingParen->is(TT_LambdaLBrace))
+            return false;
           return Style.BinPackArguments;
+        }
 
         // Ternary operator
         if (Changes[i].Tok->is(TT_ConditionalExpr))
           return true;
 
         // Period Initializer .XXX = 1.
         if (Changes[i].Tok->is(TT_DesignatedInitializerPeriod))
           return true;
 
         // Continued ternary operator
         if (Changes[i].Tok->Previous &&
             Changes[i].Tok->Previous->is(TT_ConditionalExpr))
           return true;
 
         // Continued braced list.
         if (ScopeStart > Start + 1 &&
             Changes[ScopeStart - 2].Tok->isNot(tok::identifier) &&
             Changes[ScopeStart - 1].Tok->is(tok::l_brace) &&
-            Changes[i].Tok->isNot(tok::r_brace))
+            Changes[i].Tok->isNot(tok::r_brace)) {
+          for (unsigned OuterScopeStart : llvm::reverse(ScopeStack)) {
+            // Lambda.
+            if (OuterScopeStart > Start &&
+                Changes[OuterScopeStart - 1].Tok->is(TT_LambdaLBrace))
+              return false;
+          }
           return true;
+        }
 
         return false;
       };
 
       if (ShouldShiftBeAdded())
         Changes[i].Spaces += Shift;
     }
 

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, AlignConsecutiveAssignments) {
   Alignment.ColumnLimit = 50;
   EXPECT_EQ("int x   = 0;\n"
             "int yy  = 1; /// specificlennospace\n"
             "int zzz = 2;\n",
             format("int x   = 0;\n"
                    "int yy  = 1; ///specificlennospace\n"
                    "int zzz = 2;\n",
                    Alignment));
+
+  verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {};\n"
+               "auto b                     = [] {\n"
+               "  f();\n"
+               "  return;\n"
+               "};",
+               Alignment);
+  verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {};\n"
+               "auto b                     = g([] {\n"
+               "  f();\n"
+               "  return;\n"
+               "});",
+               Alignment);
+  verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {};\n"
+               "auto b                     = g(param, [] {\n"
+               "  f();\n"
+               "  return;\n"
+               "});",
+               Alignment);
+  verifyFormat("auto aaaaaaaaaaaaaaaaaaaaa = {};\n"
+               "auto b                     = [] {\n"
+               "  if (condition) {\n"
+               "    return;\n"
+               "  }\n"
+               "};",
+               Alignment);
+
+  verifyFormat("auto b = f(aaaaaaaaaaaaaaaaaaaaaaaaa,\n"
+               "           ccc ? aaaaa : bbbbb,\n"
+               "           dddddddddddddddddddddddddd);",
+               Alignment);
+  // FIXME: https://llvm.org/PR53497
+  // verifyFormat("auto aaaaaaaaaaaa = f();\n"
+  //              "auto b            = f(aaaaaaaaaaaaaaaaaaaaaaaaa,\n"
+  //              "    ccc ? aaaaa : bbbbb,\n"
+  //              "    dddddddddddddddddddddddddd);",
+  //              Alignment);
 }
 
 TEST_F(FormatTest, AlignConsecutiveBitFields) {
   FormatStyle Alignment = getLLVMStyle();
   Alignment.AlignConsecutiveBitFields = FormatStyle::ACS_Consecutive;
   verifyFormat("int const a     : 5;\n"
                "int oneTwoThree : 23;",
                Alignment);