[clang-format] Fix numerous issues with "LambdaBodyIndentation: OuterScope" option

[jp4a50]

Summary

The previous implementation of the option involved a hack which corrupted the parenthesis state stack. The review for the previous implementation is here: https://reviews.llvm.org/D102706

Specifically, this change fixes the following github issues:

• LambdaBodyIndentation for lambda as argument argument
• clang-format 13.0.0: LambdaBodyIndentation: OuterScope closing brace & parenthesis too indented
• Inconsistent indentation closing bracket of the lambda body
• clang-format LambdaBodyIndentation=OuterScope doesn't work correctly with BraceWrapping.BeforeLambdaBody=1

This is my first contribution to clang/clang-format, so I'd appreciate some guidance.

One thing that I'd specifically like some guidance on is which version to ship these changes in and how. Although I would consider all the changes here to be bugfixes, they do considerably change the behaviour of the OuterScope option in question, so I'm not sure whether these changes are suitable to go into a patch release or if they should only go into the next major release.

[jp4a50]

clang/lib/Format/ContinuationIndenter.cpp

1132

Note that we do not apply "OuterScope" behaviour when the line's indentation level is 0 to avoid placing a lambda's closing brace at 0 indentation like this:

class Foo :
    callback{[] {
}, 
    bar,
    baz} {}

In other words, we generally try to apply this rule at function/block scope but not otherwise.

[jp4a50]

Add new code sample to demonstrate behaviour of "LambdaBodyIndentation: OuterScope"

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

3540

This code may come from Format.h

clang/unittests/Format/FormatTest.cpp

21916

Why are you changing existing tests

[owenpan]

The previous implementation of the option involved a hack which corrupted the parenthesis state stack.

Can you link the review (e.g. Dnnnnnn) of the previous implementation in the summary?

Specifically, this change fixes github issues #55708, #53212, #52846 and #59954.

Please link the issues (e.g. https://github.com/llvm/llvm-project/issues/nnnnn).

[jp4a50]

In D146042#4195295, @owenpan wrote:

The previous implementation of the option involved a hack which corrupted the parenthesis state stack.

Can you link the review (e.g. Dnnnnnn) of the previous implementation in the summary?

Specifically, this change fixes github issues #55708, #53212, #52846 and #59954.

Please link the issues (e.g. https://github.com/llvm/llvm-project/issues/nnnnn).

Done! Please see updated description.

To give a little more relevant context, my team were the original authors of this feature/option (primarily for our own benefit) so we are motivated to fix it and get it right this time.

[jp4a50]

clang/docs/ClangFormatStyleOptions.rst

3540

Sorry, I'm not sure I understand.

Do you mean that this code should *also* be added to the corresponding documentation in Format.h? Or do you mean that the documentation in this file is programatically generated from Format.h and I should therefore only put it there? Or something else?

clang/unittests/Format/FormatTest.cpp

21916

Sorry about the rather large diff here.

I have made the following changes to existing tests (all for what I think are good reasons):

• Reduced the column limit and then reduced the verbosity of the code samples in the test to make them more readable. Before the column limit was 100 and the code samples were very long. I found this made them unnecessarily difficult to read, especially because some of the lines of the code sample would wrap before a newline in the code itself, which was confusing.
• Changed a number of tests written in the form EXPECT_EQ(code, format(otherCode)) to simply verifyFormat(code) because the latter is much shorter and easier to read. If you can think of a good reason to favour the former over the latter then let me know but it just seemed like unnecessary duplication to me.
• Some of the tests weren't syntactically valid C++ e.g. the tests at line 21939 which I have changed to have a valid (rather than incomplete) trailing return type.
• The macro test actually captured a bug so I had to change the code sample there to reflect the now fixed behaviour.

I also added one or two more tests at the bottom to capture more complex cases like when more than one argument to a function call is a lambda.

[jp4a50]

clang/docs/ClangFormatStyleOptions.rst

3540

Ahh I've found dump_format_style.py now. Will fix.

[jp4a50]

Update OuterScope docs in Format.h and regenerate public docs.

[jp4a50]

All comments addressed so bump on review please :)

Also, a build has failed but I don't think my changes caused it - it looks like an old version of clang-format is being invoked which doesn't support a recently added option?

[owenpan]

Looks like this patch doesn't put the opening brace at the outerscope for some of the examples from the linked github issues:

$ cat .clang-format
AllowShortLambdasOnASingleLine: None
BraceWrapping:
  BeforeLambdaBody: true
BreakBeforeBraces: Custom
LambdaBodyIndentation: OuterScope
$ cat test.cpp
aaaaaaaaaaaaaaaaaaaa(1,
	             b(
	             	[]
                     {
                       return 0;
                     }));

some_function(a_really_long_name, another_long_name, a_third_long_name,
              [&](LineCallback line_callback)
{
  int a;
  int b;
});
$ clang-format test.cpp
aaaaaaaaaaaaaaaaaaaa(1, b(
                            []
                            {
                              return 0;
                            }));

some_function(a_really_long_name, another_long_name, a_third_long_name,
              [&](LineCallback line_callback)
              {
                int a;
                int b;
              });
$ 

Shouldn't the expected output be the same as the input?

[jp4a50]

In D146042#4204513, @owenpan wrote:

Looks like this patch doesn't put the opening brace at the outerscope for some of the examples from the linked github issues:

$ cat .clang-format
AllowShortLambdasOnASingleLine: None
BraceWrapping:
  BeforeLambdaBody: true
BreakBeforeBraces: Custom
LambdaBodyIndentation: OuterScope
$ cat test.cpp
aaaaaaaaaaaaaaaaaaaa(1,
	             b(
	             	[]
                     {
                       return 0;
                     }));

some_function(a_really_long_name, another_long_name, a_third_long_name,
              [&](LineCallback line_callback)
{
  int a;
  int b;
});
$ clang-format test.cpp
aaaaaaaaaaaaaaaaaaaa(1, b(
                            []
                            {
                              return 0;
                            }));

some_function(a_really_long_name, another_long_name, a_third_long_name,
              [&](LineCallback line_callback)
              {
                int a;
                int b;
              });
$ 

Shouldn't the expected output be the same as the input?

I'm confident that the patch will indent all those examples correctly when they are at block scope which is the only place those snippets will actually be valid code. I added an exception (see comment in ContinuationIndenter.cpp) to the code to disable OuterScope's behaviour when the line's indentation level is 0 (i.e. statements at namespace scope) because otherwise you can end up with things like:

Namespace::Foo::Foo(
  Arg arg1, Arg arg2,
  Arg arg3, Arg arg4)
  : init1{arg1},
    init2{[arg2]() {
  return arg2;
}},
    init3{arg3},
    init4{arg4} {}

The main rationale behind the OuterScope style is that the lambda body will be indented similarly to the code block of an if statement (or for etc) inside function definitions/blocks, but this breaks down when you have something like an constructor initializer which lives outside of a code block. The way I've handled this case isn't particularly elegant, though, so I'd be happy to remove the relevant code from this diff as it's not necessary to fix the issues I've linked - it was an extra bit of behaviour that my team (authors of the original implementation of OuterScope) wanted.

FYI I will be away until next Monday now but thanks for the feedback so far.

[MyDeveloperDay]

clang/unittests/Format/FormatTest.cpp

21916

refactoring the tests is one thing, but not at the same time you are modifying how they are formatted. Could it be a separate commit so we can actually see what is changing?

I don't deny what is there before doesn't look 100% correct, but I've always worked on the Beyoncé rule (If you liked it you should have put a test on it), meaning... someone put a test there to assert some form of formatting behaviour, you are now changing that to be your desired behaviour, that break the rule (IMHO)

we have to agree what was there before wasn't correct, but your changes don't give me that confidence,

21918–21927

This is a completely different function what happen to the std::vector<int> argument?

21952–21953

I feel like lots of these test changes could be made on their own, BEFORE changing the code, I would feel more comfortable about that I think

21953

I would think the point here is to test someLongArgumentName not to use a small variable like 'v'

[jp4a50]

clang/unittests/Format/FormatTest.cpp

21916

refactoring the tests is one thing, but not at the same time you are modifying how they are formatted. Could it be a separate commit so we can actually see what is changing?

Sure! I assume that this implies two separate Phabricator reviews/diffs? Or is there somehow a way to show two "commits" within a single review? Sorry, I'm not used to using Phrabricator.

I don't deny what is there before doesn't look 100% correct, but I've always worked on the Beyoncé rule (If you liked it you should have put a test on it), meaning... someone put a test there to assert some form of formatting behaviour, you are now changing that to be your desired behaviour, that break the rule (IMHO)

The macro example I was referring to as a "bug" actually had a comment below it which seemed to acknowledge an issue with the previous implementation. Now I'm not 100% sure that that comment was referring to the behaviour in the macro example but what does seem clear is that the behaviour shown in the macro test case is not desirable in the case of using OuterScope. I just can't see how that behaviour could be justified as being intentional/desirable. If you wanted, I could reach out to the original author of that test and ask them since they still work at the same company.

we have to agree what was there before wasn't correct, but your changes don't give me that confidence,

I'd appreciate if you could elaborate on this - perhaps you're referring mainly to the macro example but I'm not sure.

21918–21927

The changes to shorten various types and variable names was to avoid lines (in the code sample) being broken onto multiple newlines where they weren't before because I changed the column limit from 100 to 60 (so leaving the code sample changed would have led to additional line breaks).

I changed the column limit from 100 to 60 purely because I thought the 100 column limit made the tests unnecessarily verbose - there were lots of characters to read for seemingly no benefit.

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

21916

Each commit gets its own differential, there are no multiple commit reviews like pull requests. You can add a parent or child commit to show the dependency, if any.

[jp4a50]

Rebase diff on top of D146995.

[jp4a50]

clang/unittests/Format/FormatTest.cpp

21916

Pulled out test refactoring (excluding functional changes) into D146995.

21952–21953

Pulled them out into D146995.

[jp4a50]

Re-enable OuterScope lambda body indentation of lambdas in namespace scope statements.

[jp4a50]

I've removed the special-casing that I added for namespace scope statements. If I make those changes at all, I'll raise them as a follow up diff.

clang/unittests/Format/FormatTest.cpp

21953

These changes have been moved to D146995 now, but I'll respond here: the long variable names were (presumably) being used to cause a line break. Since I've reduced the column limit from 100 to 60, v is now sufficient (and less noisy to read).

[jp4a50]

Could I get a re-review on this one please?

[owenpan]

In D146042#4204651, @jp4a50 wrote:

I'm confident that the patch will indent all those examples correctly when they are at block scope which is the only place those snippets will actually be valid code. I added an exception (see comment in ContinuationIndenter.cpp) to the code to disable OuterScope's behaviour when the line's indentation level is 0 (i.e. statements at namespace scope) because otherwise you can end up with things like:

Namespace::Foo::Foo(
  Arg arg1, Arg arg2,
  Arg arg3, Arg arg4)
  : init1{arg1},
    init2{[arg2]() {
  return arg2;
}},
    init3{arg3},
    init4{arg4} {}

Sorry that I missed it.

clang/include/clang/Format/Format.h

2652–2656 ↗

(On Diff #508765)

We might as well clean it up a bit.

clang/unittests/Format/FormatTest.cpp

21990

Ditto below.

[jp4a50]

Tidy up docs and tests.

[jp4a50]

In D146042#4239182, @owenpan wrote:

In D146042#4204651, @jp4a50 wrote:

I'm confident that the patch will indent all those examples correctly when they are at block scope which is the only place those snippets will actually be valid code. I added an exception (see comment in ContinuationIndenter.cpp) to the code to disable OuterScope's behaviour when the line's indentation level is 0 (i.e. statements at namespace scope) because otherwise you can end up with things like:

Namespace::Foo::Foo(
  Arg arg1, Arg arg2,
  Arg arg3, Arg arg4)
  : init1{arg1},
    init2{[arg2]() {
  return arg2;
}},
    init3{arg3},
    init4{arg4} {}

Sorry that I missed it.

NP - as I say, I've removed this special casing for now since it is controversial to include it in a patch claiming to simply fix bugs.

[jp4a50]

Add release note.

[jp4a50]

Thanks for accepting!

Commit details as follows as per previous diff:

Name: Jon Phillips
Email: [e-mail]

If someone could commit for me that would be much appreciated.

Edit: also note that there are 4 issues that I believe we can close when this is committed, as noted in the description.

clang/docs/ClangFormatStyleOptions.rst

* ``LBI_OuterScope`` (in configuration: ``OuterScope``)
 
     .. code-block:: c++
 
        someMethod(
            [](SomeReallyLongLambdaSignatureArgument foo) {
          return;
        });
 
+       someMethod(someOtherMethod(
+           [](SomeReallyLongLambdaSignatureArgument foo) {
+         return;
+       }));
+
 
 
 .. _Language:
 
 **Language** (``LanguageKind``) :versionbadge:`clang-format 3.5` :ref:`¶ <Language>`
   Language, this format style is targeted at.
 
   Possible values:

clang/lib/Format/ContinuationIndenter.cpp

unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
   // thus are on the same line as the current token.
   if ((Style.isVerilog() && Keywords.isVerilogEndOfLabel(Previous)) ||
       (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths &&
        State.Line->First->is(tok::kw_enum))) {
     return (Style.IndentWidth * State.Line->First->IndentLevel) +
            Style.IndentWidth;
   }
 
-  if (NextNonComment->is(tok::l_brace) && NextNonComment->is(BK_Block))
-    return Current.NestingLevel == 0 ? State.FirstIndent : CurrentState.Indent;
+  if (NextNonComment->is(tok::l_brace) && NextNonComment->is(BK_Block)) {
+    if (Current.NestingLevel == 0 ||
+        (Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
+         State.NextToken->is(TT_LambdaLBrace) && State.Line &&
+         State.Line->Level != 0)) {
+      return State.FirstIndent;
+    }
+    return CurrentState.Indent;
+  }
   if ((Current.isOneOf(tok::r_brace, tok::r_square) ||
        (Current.is(tok::greater) &&
         (Style.Language == FormatStyle::LK_Proto ||
          Style.Language == FormatStyle::LK_TextProto))) &&
       State.Stack.size() > 1) {
     if (Current.closesBlockOrBlockTypeList(Style))
       return State.Stack[State.Stack.size() - 2].NestedBlockIndent;
     if (Current.MatchingParen && Current.MatchingParen->is(BK_BracedInit))
if (Current.is(tok::r_square)) {
     // If this ends the array subscript expr, reset the corresponding value.
     const FormatToken *NextNonComment = Current.getNextNonComment();
     if (NextNonComment && NextNonComment->isNot(tok::l_square))
       CurrentState.StartOfArraySubscripts = 0;
   }
 }
 
 void ContinuationIndenter::moveStateToNewBlock(LineState &State) {
+  if (Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
+      State.NextToken->is(TT_LambdaLBrace) && State.Line &&
+      State.Line->Level != 0) {
+    State.Stack.back().NestedBlockIndent = State.FirstIndent;
+  }
   unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent;
   // ObjC block sometimes follow special indentation rules.
   unsigned NewIndent =
       NestedBlockIndent + (State.NextToken->is(TT_ObjCBlockLBrace)
                                ? Style.ObjCBlockIndentWidth
                                : Style.IndentWidth);
   State.Stack.push_back(ParenState(State.NextToken, NewIndent,
                                    State.Stack.back().LastSpace,

clang/lib/Format/UnwrappedLineFormatter.cpp

if (TheLine->Last->is(tok::l_brace)) {
         ShouldMerge = Style.AllowShortEnumsOnASingleLine;
       } else if (TheLine->Last->isOneOf(TT_ClassLBrace, TT_StructLBrace)) {
         // NOTE: We use AfterClass (whereas AfterStruct exists) for both classes
         // and structs, but it seems that wrapping is still handled correctly
         // elsewhere.
         ShouldMerge = !Style.BraceWrapping.AfterClass ||
                       (NextLine.First->is(tok::r_brace) &&
                        !Style.BraceWrapping.SplitEmptyRecord);
-      } if(TheLine->InPPDirective ||
-               !TheLine->First->isOneOf(tok::kw_class, tok::kw_enum,
-                                        tok::kw_struct)) {
+      }
+      if (TheLine->InPPDirective ||
         // Try to merge a block with left brace unwrapped that wasn't yet
         // covered.
         ShouldMerge = !Style.BraceWrapping.AfterFunction ||
                       (NextLine.First->is(tok::r_brace) &&
                        !Style.BraceWrapping.SplitEmptyFunction);
       }
       return ShouldMerge ? tryMergeSimpleBlock(I, E, Limit) : 0;
     }
if (Previous.Children.size() == 0 || (!HasLBrace && !LBrace->MacroParent)) {
       return true;
     }
 
     if (NewLine || Previous.MacroParent) {
       const ParenState &P = State.Stack.back();
 
       int AdditionalIndent =
           P.Indent - Previous.Children[0]->Level * Style.IndentWidth;
-
-      if (Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
-          P.NestedBlockIndent == P.LastSpace) {
-        if (State.NextToken->MatchingParen &&
-            State.NextToken->MatchingParen->is(TT_LambdaLBrace)) {
-          State.Stack.pop_back();
-        }
-        if (LBrace->is(TT_LambdaLBrace))
-          AdditionalIndent = 0;
-      }
-
       Penalty +=
           BlockFormatter->format(Previous.Children, DryRun, AdditionalIndent,
                                  /*FixBadIndentation=*/true);
       return true;
     }
 
     if (Previous.Children[0]->First->MustBreakBefore)
       return false;

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, FormatsLambdas) {
   verifyFormat("namespace test {\n"
                "class Test {\n"
                "public:\n"
                "  Test() = default;\n"
                "};\n"
                "} // namespace test",
                LLVMWithBeforeLambdaBody);
 
   // Lambdas with different indentation styles.
-  Style = getLLVMStyleWithColumns(100);
-  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+  Style = getLLVMStyleWithColumns(60);
+  verifyFormat("Result doSomething(Promise promise) {\n"
                "  return promise.then(\n"
-            "      [this, &someVariable, someObject = "
-            "std::mv(s)](std::vector<int> evaluated) mutable {\n"
+               "      [this, obj = std::move(s)](int evaluated) mutable {\n"
                "        return someObject.startAsyncAction().then(\n"
-            "            [this, &someVariable](AsyncActionResult result) "
-            "mutable { result.processMore(); });\n"
+               "            [this, &obj](Result result) mutable {\n"
+               "              result.processMore();\n"
+               "            });\n"
                "      });\n"
                "}\n",
-            format("SomeResult doSomething(SomeObject promise) {\n"
-                   "  return promise.then([this, &someVariable, someObject = "
-                   "std::mv(s)](std::vector<int> evaluated) mutable {\n"
-                   "    return someObject.startAsyncAction().then([this, "
-                   "&someVariable](AsyncActionResult result) mutable {\n"
-                   "      result.processMore();\n"
-                   "    });\n"
-                   "  });\n"
-                   "}\n",
-                   Style));
-  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
-  verifyFormat("test() {\n"
-               "  ([]() -> {\n"
+               Style);
+  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
+  verifyFormat("Result doSomething(Promise promise) {\n"
+               "  return promise.then(\n"
+               "      [this, obj = std::move(s)](int bar) mutable {\n"
+               "    return obj.startAsyncAction().then(\n"
+               "        [this, &obj](Result result) mutable {\n"
+               Style);
+  verifyFormat("void test() {\n"
+               "  ([]() -> auto {\n"
                "    int b = 32;\n"
                "    return 3;\n"
                "  }).foo();\n"
                "}",
                Style);
-  verifyFormat("test() {\n"
-               "  []() -> {\n"
+  verifyFormat("void test() {\n"
+               "  []() -> auto {\n"
                "    int b = 32;\n"
                "    return 3;\n"
                "  }\n"
                "}",
                Style);
-  verifyFormat("std::sort(v.begin(), v.end(),\n"
-               "          [](const auto &someLongArgumentName, const auto "
-               "&someOtherLongArgumentName) {\n"
-               "  return someLongArgumentName.someMemberVariable < "
-               "someOtherLongArgumentName.someMemberVariable;\n"
-               "});",
+  verifyFormat("void test() {\n"
+               "  std::sort(v.begin(), v.end(),\n"
+               "            [](const auto &foo, const auto &bar) {\n"
+               "    return foo.baz < bar.baz;\n"
+               "  });\n"
+               "}\n",
                Style);
-  verifyFormat("test() {\n"
+  verifyFormat("void test() {\n"
                "  (\n"
-               "      []() -> {\n"
-               "        int b = 32;\n"
-               "        return 3;\n"
-               "      },\n"
+               "      []() -> auto {\n"
+               "    int b = 32;\n"
+               "    return 3;\n"
+               "  },\n"
                "      foo, bar)\n"
                "      .foo();\n"
                "}",
                Style);
-  verifyFormat("test() {\n"
-               "  ([]() -> {\n"
+  verifyFormat("void test() {\n"
+               "  ([]() -> auto {\n"
                "    int b = 32;\n"
                "    return 3;\n"
                "  })\n"
                "      .foo()\n"
                "      .bar();\n"
                "}",
                Style);
-  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
-            "  return promise.then(\n"
-            "      [this, &someVariable, someObject = "
-            "std::mv(s)](std::vector<int> evaluated) mutable {\n"
-            "    return someObject.startAsyncAction().then(\n"
-            "        [this, &someVariable](AsyncActionResult result) mutable { "
-            "result.processMore(); });\n"
-            "  });\n"
-            "}\n",
-            format("SomeResult doSomething(SomeObject promise) {\n"
-                   "  return promise.then([this, &someVariable, someObject = "
-                   "std::mv(s)](std::vector<int> evaluated) mutable {\n"
-                   "    return someObject.startAsyncAction().then([this, "
-                   "&someVariable](AsyncActionResult result) mutable {\n"
-                   "      result.processMore();\n"
-                   "    });\n"
-                   "  });\n"
-                   "}\n",
-                   Style));
-  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
-            "  return promise.then([this, &someVariable] {\n"
-            "    return someObject.startAsyncAction().then(\n"
-            "        [this, &someVariable](AsyncActionResult result) mutable { "
-            "result.processMore(); });\n"
-            "  });\n"
+  verifyFormat("void foo() {\n"
+               "  aFunction(1, b(c(foo, bar, baz, [](d) {\n"
+               "    auto f = e(d);\n"
+               "    return f;\n"
+               "  })));\n"
+               Style);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  verifyFormat("void foo() {\n"
+               "  aFunction(\n"
+               "      1, b(c(\n"
+               "             [](d) -> Foo {\n"
+               "    auto f = e(d);\n"
+               "    return f;\n"
+               "  },\n"
+               "             foo, Bar{},\n"
+               "             [] {\n"
+               "    auto g = h();\n"
+               "    return g;\n"
+               "  },\n"
+               "             baz)));\n"
                "}\n",
-            format("SomeResult doSomething(SomeObject promise) {\n"
-                   "  return promise.then([this, &someVariable] {\n"
-                   "    return someObject.startAsyncAction().then([this, "
-                   "&someVariable](AsyncActionResult result) mutable {\n"
-                   "      result.processMore();\n"
-                   "    });\n"
-                   "  });\n"
-                   "}\n",
-                   Style));
-  Style = getGoogleStyle();
-  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
-  EXPECT_EQ("#define A                                       \\\n"
-            "  [] {                                          \\\n"
-            "    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(        \\\n"
-            "        xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx); \\\n"
-            "      }",
-            format("#define A [] { xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx( \\\n"
-                   "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx); }",
-                   Style));
-  // TODO: The current formatting has a minor issue that's not worth fixing
-  // right now whereby the closing brace is indented relative to the signature
-  // instead of being aligned. This only happens with macros.
+               Style);
+  verifyFormat("void foo() {\n"
+               "  aFunction(1, b(c(foo, Bar{}, baz, [](d) -> Foo {\n"
+               "    auto f = e(\n"
+               "        foo,\n"
+               "        [&] {\n"
+               "      auto g = h();\n"
+               "      return g;\n"
+               "    },\n"
+               "        qux,\n"
+               "        [&] -> Bar {\n"
+               "      auto i = j();\n"
+               "      return i;\n"
+               "    });\n"
+               "    return f;\n"
+               "  })));\n"
+               Style);
+  verifyFormat("#define A                                                  \\\n"
+               "  [] {                                                     \\\n"
+               "    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(                   \\\n"
+               "        xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx);            \\\n"
+               "  }",
+               Style);
+  verifyFormat("Namespace::Foo::Foo(\n"
+               "    LongClassName bar, AnotherLongClassName baz)\n"
+               "    : baz{baz}, func{[&] {\n"
+               "        auto qux = bar;\n"
+               "        return aFunkyFunctionCall(qux);\n"
+               "      }} {}\n",
+               Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.BeforeLambdaBody = true;
+  verifyFormat("void foo() {\n"
+               "  aFunction(\n"
+               "      1, b(c(foo, Bar{}, baz,\n"
+               "             [](d) -> Foo\n"
+               "  {\n"
+               "    auto f = e(\n"
+               "        [&]\n"
+               "    {\n"
+               "      auto g = h();\n"
+               "      return g;\n"
+               "    },\n"
+               "        qux,\n"
+               "        [&] -> Bar\n"
+               "    {\n"
+               "      auto i = j();\n"
+               "      return i;\n"
+               "    });\n"
+               "    return f;\n"
+               "  })));\n"
+               "}\n",
+               Style);
 }
 
 TEST_F(FormatTest, LambdaWithLineComments) {
   FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
   LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
   LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
   LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
       FormatStyle::ShortLambdaStyle::SLS_All;