[clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

[MyDeveloperDay]

Summary

https://bugs.llvm.org/show_bug.cgi?id=50702

I believe D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style) may be too aggressive with brace wrapping rules which doesn't always apply to Lamdbas

The introduction of BeforeLambdaBody and AllowShortLambdasOnASingleLine has impact on brace handling on other block types, which I suspect we didn't see before as people may not be using the BeforeLambdaBody style

From what I can tell this can be seen by the unit test I change as its not honouring the orginal LLVM brace wrapping style for the Fct() function

I added a unit test from PR50702 and have removed some of the code (which has zero impact on the unit test, which kind of suggests its unnecessary), some additional attempt has been made to try and ensure we'll only break on what is actually a LamdbaLBrace

[Wawha]

clang/lib/Format/TokenAnnotator.cpp

3760

Without this code, you should be able to remove functions isAllmanBraceIncludedBreakableLambda(), isItAInlineLambdaAllowed() and isOneChildWithoutMustBreakBefore().

Thank you for the fix.

When I implement that part, I have some difficulty to handle inline lambda and BreakBeforeLambdaBody. At the beginning, I implement the solution without inline lambda option (not yet present). But I have to handle it before merging the patch. And, this modification was added at this moment.

[MyDeveloperDay]

@Wawha do you have a project that perhaps uses this work your did? I would like to ensure I didn't break anything

[MyDeveloperDay]

Remove unused functions

[lichray]

LGTM

[Wawha]

In D104222#2817147, @MyDeveloperDay wrote:

@Wawha do you have a project that perhaps uses this work your did? I would like to ensure I didn't break anything

@MyDeveloperDay Good idea!
I just test on my code, and I see 3 types of regressions due to that patch.
This code:

		auto select = [this]() -> const Library::Object*
		{
			return MyAssignment::SelectFromList(this);
		};

Become:

		auto select = [this]() -> const Library::Object* {
			return MyAssignment::SelectFromList(this);
		};

And this code,

	auto Items()
	{
		return iter::imap(
			[this](const std::unique_ptr<ItemBase>& iItem) -> T&
			{
				return *GetItem(*iItem)->Item;
			},
			m_Root->Items());
	}

Become:

	auto Items()
	{
		return iter::imap(
			[this](const std::unique_ptr<ItemBase>& iItem) -> T& {
				return *GetItem(*iItem)->Item;
			},
			m_Root->Items());
	}

And the last case is due to inline lambda. Even if the option is disable, for that code

		auto AllParents() const
		{
			return iter::chain.from_iterable(iter::imap(
				[](auto&& ite) -> auto&
				{
					return ite.second;
				},
				m_Parents));
		}

It's format like this:

		auto AllParents() const
		{
			return iter::chain.from_iterable(iter::imap(
				[](auto&& ite) -> auto& { return ite.second; }, m_Parents));
		}

I'm surprise, because the 3 cases, should be present inside the UnitTest.

[christophe-calmejane]

Hi,

Due to the pandemic, I wasn't able to look at https://reviews.llvm.org/D44609 which got broken since it was pushed to LLVM. Nevertheless I commented in that post with a rather complete unit tests that should pass if any changes are to be pushed to that part of the code.

Here is the test suite:

noOtherParams([](int x){call();});
paramBeforeLambda(8,[](int x){call();});
paramAfterLambda([](int x){call();},5);
paramBeforeAndAfterLambda(8,[](int x){call();},5);
doubleLambda(8,[](int x){call();},6,[](float f){return f*2;},5);
nestedLambda1([](int x){return [](float f){return f*2;};});
nestedLambda2([](int x){ call([](float f){return f*2;});});
noExceptCall([](int x) noexcept {call();});
mutableCall([](int x) mutable{call();});

funcCallFunc(call(),[](int x){call();});

auto const l=[](char v){if(v)call();};
void f() { return [](char v){ if(v) return v++;}; }

And here is how it should be formatted:

noOtherParams(
        [](int x)
        {
                call();
        });
paramBeforeLambda(8,
        [](int x)
        {
                call();
        });
paramAfterLambda(
        [](int x)
        {
                call();
        },
        5);
paramBeforeAndAfterLambda(8,
        [](int x)
        {
                call();
        },
        5);
doubleLambda(8,
        [](int x)
        {
                call();
        },
        6,
        [](float f)
        {
                return f * 2;
        },
        5);
nestedLambda1(
        [](int x)
        {
                return [](float f)
                {
                        return f * 2;
                };
        });
nestedLambda2(
        [](int x)
        {
                call(
                        [](float f)
                        {
                                return f * 2;
                        });
        });
noExceptCall(
        [](int x) noexcept
        {
                call();
        });
mutableCall(
        [](int x) mutable
        {
                call();
        });

funcCallFunc(call(),
        [](int x)
        {
                call();
        });

auto const l = [](char v)
{
        if (v)
                call();
};
void f()
{
        return [](char v)
        {
                if (v)
                        return v++;
        };
}

I couldn't find the time to test this case with the latest clang-format release, but I guess it's still broken and doesn't match the expected output.
Anyway, this test suite is helpful as it covers most cases, and if someone with enough knowledge of LLVM is willing to add this to the unit tests it would help a lot.

Best regards,
Chris

[Wawha]

Hi @christophe-calmejane.
I test your code with a old version (few time after the merge of https://reviews.llvm.org/D44609), and the latest version (commit: e0c382a9d5a0), and in both cases I have the same result, which is near your output:

noOtherParams(
	[](int x)
	{
		call();
	});
paramBeforeLambda(
	8,
	[](int x)
	{
		call();
	});
paramAfterLambda(
	[](int x)
	{
		call();
	},
	5);
paramBeforeAndAfterLambda(
	8,
	[](int x)
	{
		call();
	},
	5);
doubleLambda(
	8,
	[](int x)
	{
		call();
	},
	6,
	[](float f)
	{
		return f * 2;
	},
	5);
nestedLambda1(
	[](int x)
	{
		return [](float f)
		{
			return f * 2;
		};
	});
nestedLambda2(
	[](int x)
	{
		call(
			[](float f)
			{
				return f * 2;
			});
	});
noExceptCall(
	[](int x) noexcept
	{
		call();
	});
mutableCall(
	[](int x) mutable
	{
		call();
	});

funcCallFunc(
	call(),
	[](int x)
	{
		call();
	});

auto const l = [](char v)
{
	if(v)
		call();
};
void f()
{
	return [](char v)
	{
		if(v)
			return v++;
	};
}

The only different is for cases with multiples parameters.

paramBeforeLambda(
	8,
	[](int x)
	{
		call();
	}

I though I add multiples cases, but looking at UnitTests with inline lambda "None", I see only few tests, perhaps some are missing.

[christophe-calmejane]

In D104222#2819324, @Wawha wrote:

Hi @christophe-calmejane.
I test your code with a old version (few time after the merge of https://reviews.llvm.org/D44609), and the latest version (commit: e0c382a9d5a0), and in both cases I have the same result, which is near your output:

I though I add multiples cases, but looking at UnitTests with inline lambda "None", I see only few tests, perhaps some are missing.

I posted a fix for the incorrect formatting you have in your output (in your original review post), but like I said back then, I thought the fix was not clean enough to be included. Nevertheless it did fix all cases and formatting was perfect for my whole test suite.
I using this "patched" version since then as it's the only one with the correct lambda formatting. I tried to reapply my patch on a recent version.. of course it failed as there were too many changes in clang-format. I don't have enough knowledge on this project to be able to clearly understand how it works and fix the formatting on the latest version (but I wish I could)... at least not without having to spend too much time on it :(
For the record, my patch over clang-format 7.0 can be found here: https://www.kikisoft.com/Hive/clang-format/

[MyDeveloperDay]

@Wawha your cases could be covered by the following (in mustBreakBefore)

if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace) &&
    Left.isOneOf(tok::star, tok::amp, tok::ampamp)) {
  return true;
}

As I think its the presence of & and * that causes it to not wrap.

By all means if you would prefer to commandeer the patch I'm ok with that. I was just on a bug fixing day. If you'd like to take this over and add the unit tests @christophe-calmejane suggest, I'm happy to be the reviewer

[MyDeveloperDay]

@christophe-calmejane are you able to post your .clang-format styles that you are using, I'm struggling to see where its not matching your style (other than brace styles on the function and argument placing)

[MyDeveloperDay]

Add some of the failing cases identified by @Wawha

[Wawha]

In D104222#2819358, @MyDeveloperDay wrote:

@Wawha your cases could be covered by the following (in mustBreakBefore)

if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace) &&
    Left.isOneOf(tok::star, tok::amp, tok::ampamp)) {
  return true;
}

As I think its the presence of & and * that causes it to not wrap.

By all means if you would prefer to commandeer the patch I'm ok with that. I was just on a bug fixing day. If you'd like to take this over and add the unit tests @christophe-calmejane suggest, I'm happy to be the reviewer

Indeed, & and * could be the root cause.
About fixing that, I won't have the time in the next days. It will take time for me to re-read and re-understand that code.
But I could find time to test patch or modification in my code to check that there is no regression.

[Wawha]

In D104222#2819422, @MyDeveloperDay wrote:

Add some of the failing cases identified by @Wawha

Thank for the fix. I test and discover a new case with a regression:

	auto createObj = [this] -> std::unique_ptr<Object>
		{
		  return std::make_unique<Object>();
		};

Like & and * a template return ending with > is not handle.

[MyDeveloperDay]

Add support for TemplateCloser before LambdaBrace

[HazardyKnusperkeks]

Just asking: Would it be different if there is auto, decltype(auto), decltype(a+b), typename or template as trailing return type?

[Wawha]

Indeed, I found another case with a regression:

return iter::chain.from_iterable(
  [](auto&& ite) -> auto&
  {
    return ite.second;
  });

it's format all inline.

And also another different in that case (without lambda):

	ASSERT_NO_THROW(
		{
			iterator += 507408;
		});

is now format like this:

	ASSERT_NO_THROW({ iterator += 507408; });

[MyDeveloperDay]

Part of the problem here is we are not really parsing the Lambda but trying to break based on token annotation only. This can tend to be a bit fragile (normally around corner cases)

The previous approach I think was not constrained enough to just lambdas which is why we had the bleed into other things, also when we added this we didn't really test that it didn't have any impact on anything else.

Actually this has been a problem when adding new options. I keep meaning to extend the LLVMDefaultStyle GNUDefaultStyle MozillaDefaultStyle tests to cover all behaviour (if/else/while/do/for/etc..) but also to find a way that when a new option is added we can validate that non of the other aspects of a style aren't broken when using that. (not an easy problem to fix)

I'm keen for this NOT to break the AfterClass, AfterNamespace issue, (which is a regression)... we can continue to chip away at corner cases or revert the original change completely. (which I also don't want to do)

likely trying to add add parseXXX() functions in UnwrappedLineParser would likely take much longer. I feel like that might offer the better solution but think I would prefer we used that to replace this and gravitate slowly towards a more correct case

[MyDeveloperDay]

And also another different in that case (without lambda):

	ASSERT_NO_THROW(
		{
			iterator += 507408;
		});

is now format like this:

	ASSERT_NO_THROW({ iterator += 507408; });

I think this shows where your change made an assumption that this was a lambda, but its not necessarily is as there is no TT_LamdbaLBrace, I would argue your change was the regression even if looks nicer (in your view)

AnnotatedTokens(L=0):
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=identifier L=15 PPK=2 FakeLParens= FakeRParens=0 II=0xad8a50 Text='ASSERT_NO_THROW'
 M=0 C=0 T=Unknown S=0 F=0 B=1 BK=0 P=23 Name=l_paren L=16 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=1 P=140 Name=l_brace L=17 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='{'
 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=61 Name=r_brace L=39 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='}'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=40 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=semi L=41 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=';'

[MyDeveloperDay]

For this case:

return iter::chain.from_iterable(
  [](auto&& ite) -> auto&
  {
    return ite.second;
  });

The code has not been classified as a Lambda

AnnotatedTokens(L=0):
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=return L=6 PPK=2 FakeLParens= FakeRParens=0 II=0x24bf7e8 Text='return'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=23 Name=identifier L=11 PPK=2 FakeLParens=0/ FakeRParens=0 II=0x24c35c0 Text='iter'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=coloncolon L=13 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=520 Name=identifier L=18 PPK=2 FakeLParens= FakeRParens=0 II=0x24c3738 Text='chain'
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=170 Name=period L=19 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='.'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=identifier L=32 PPK=2 FakeLParens= FakeRParens=0 II=0x24c3760 Text='from_iterable'
 M=0 C=0 T=Unknown S=0 F=0 B=1 BK=0 P=23 Name=l_paren L=33 PPK=1 FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=1 T=ArrayInitializerLSquare S=0 F=0 B=0 BK=0 P=59 Name=l_square L=34 PPK=2 FakeLParens=1/ FakeRParens=0 II=0x0 Text='['
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=r_square L=35 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=']'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=36 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=auto L=40 PPK=2 FakeLParens= FakeRParens=0 II=0x24bf510 Text='auto'
 M=0 C=1 T=PointerOrReference S=1 F=0 B=0 BK=0 P=250 Name=ampamp L=43 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='&&'
 M=0 C=1 T=StartOfName S=0 F=0 B=0 BK=0 P=260 Name=identifier L=46 PPK=2 FakeLParens= FakeRParens=0 II=0x24c3780 Text='ite'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=63 Name=r_paren L=47 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 M=0 C=1 T=LambdaArrow S=1 F=0 B=0 BK=0 P=150 Name=arrow L=50 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='->'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=43 Name=auto L=55 PPK=2 FakeLParens= FakeRParens=0 II=0x24bf510 Text='auto'
 M=0 C=1 T=PointerOrReference S=1 F=0 B=0 BK=0 P=230 Name=amp L=57 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='&'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=1 P=48 Name=l_brace L=59 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='{'
 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=61 Name=r_brace L=80 PPK=2 FakeLParens= FakeRParens=1 II=0x0 Text='}'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=81 PPK=2 FakeLParens= FakeRParens=1 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=semi L=82 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=';'

It doesn't see this

M=0 C=0 T=Unknown S=1 F=0 B=0 BK=1 P=48 Name=l_brace L=59 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='{'

As a LamdaLBrace

Again I would say this was actually a failure elsewhere

[MyDeveloperDay]

In D104222#2820001, @HazardyKnusperkeks wrote:

Just asking: Would it be different if there is auto, decltype(auto), decltype(a+b), typename or template as trailing return type?

It would be good if you could give a small example, just so I can clarify what you mean,

but I think these could easily be corner cases which at least for a current non parsed based solution would could potentially fill with mustBreak rules..

[christophe-calmejane]

In D104222#2819403, @MyDeveloperDay wrote:

@christophe-calmejane are you able to post your .clang-format styles that you are using, I'm struggling to see where its not matching your style (other than brace styles on the function and argument placing)

@MyDeveloperDay
Of course, sorry for the delay, here it is: https://github.com/christophe-calmejane/Hive/blob/master/.clang-format

[MyDeveloperDay]

@MyDeveloperDay
Of course, sorry for the delay, here it is: https://github.com/christophe-calmejane/Hive/blob/master/.clang-format

You don't have AllowShortLambdasOnASingleLine set to be false or None

If you did The only difference (with this current patch) would be the positioning of the first arguments on paramBeforeAndAfterLambda and doubleLambda (I'm not sure the reason for why that is)

[christophe-calmejane]

In D104222#2821375, @MyDeveloperDay wrote:

@MyDeveloperDay
Of course, sorry for the delay, here it is: https://github.com/christophe-calmejane/Hive/blob/master/.clang-format

You don't have AllowShortLambdasOnASingleLine set to be false or None

Just a clarification, this file is meant to be used with clang-format v7.0.0 along with my small patch fixing formatting issues that were not included in the original merge (see the other post). This means it's using the defaults from back then ;)
I never had a chance to upgrade clang-format because lambda formatting was broken (some of my test cases were failing). I will try to build your latest patch and see how it goes. I would expect my project re-formatting to not change much :D
Thanks for your work btw.

[christophe-calmejane]

If you did The only difference (with this current patch) would be the positioning of the first arguments on paramBeforeAndAfterLambda and doubleLambda (I'm not sure the reason for why that is)

@MyDeveloperDay The reason is the fix I talked about in the other thread (https://reviews.llvm.org/D44609) that I proposed for LLVM 7.0.0. But last time I checked, my patch couldn't be applied anymore due to massive changes in the code. I don't have enough knowledge of clang-format to clearly follow where went the code my patch applies to, I originally fixed the issue blindly by stepping into the code :)
Nevertheless, I think the issue is still present (since my patch was not applied to the base code) and it might still be related to this reason. See my comment that fixed the issue back then (https://reviews.llvm.org/D44609#1255395).

It would be really awesome if this could finally be fixed so we have a consistent formatting for lambdas!

clang/lib/Format/TokenAnnotator.cpp

 }
 
 static bool
 isItAnEmptyLambdaAllowed(const FormatToken &Tok,
                          FormatStyle::ShortLambdaStyle ShortLambdaOption) {
   return Tok.Children.empty() && ShortLambdaOption != FormatStyle::SLS_None;
 }
 
-static bool
-isItAInlineLambdaAllowed(const FormatToken &Tok,
-                         FormatStyle::ShortLambdaStyle ShortLambdaOption) {
-  return (ShortLambdaOption == FormatStyle::SLS_Inline &&
-          IsFunctionArgument(Tok)) ||
-         (ShortLambdaOption == FormatStyle::SLS_All);
-}
-
-static bool isOneChildWithoutMustBreakBefore(const FormatToken &Tok) {
-  if (Tok.Children.size() != 1)
-    return false;
-  FormatToken *curElt = Tok.Children[0]->First;
-  while (curElt) {
-    if (curElt->MustBreakBefore)
-      return false;
-    curElt = curElt->Next;
-  }
-  return true;
-}
 static bool isAllmanLambdaBrace(const FormatToken &Tok) {
   return (Tok.is(tok::l_brace) && Tok.is(BK_Block) &&
           !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral));
 }
 
-static bool isAllmanBraceIncludedBreakableLambda(
-    const FormatToken &Tok, FormatStyle::ShortLambdaStyle ShortLambdaOption) {
-  if (!isAllmanLambdaBrace(Tok))
-    return false;
-
-  if (isItAnEmptyLambdaAllowed(Tok, ShortLambdaOption))
-    return false;
-
-  return !isItAInlineLambdaAllowed(Tok, ShortLambdaOption) ||
-         !isOneChildWithoutMustBreakBefore(Tok);
-}
-
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
                                      const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
     return true;
 
   if (Style.isCSharp()) {
     if (Right.is(TT_CSharpNamedArgumentColon) ||
if ((Right.Previous->is(tok::l_brace) ||
       Right.NestingLevel == 1 && Style.Language == FormatStyle::LK_Proto) {
     // Don't put enums or option definitions onto single lines in protocol
     // buffers.
     return true;
   }
   if (Right.is(TT_InlineASMBrace))
     return Right.HasUnescapedNewline;
 
-  auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
-  if (Style.BraceWrapping.BeforeLambdaBody &&
-      (isAllmanBraceIncludedBreakableLambda(Left, ShortLambdaOption) ||
-       isAllmanBraceIncludedBreakableLambda(Right, ShortLambdaOption))) {
-    return true;
-  }
-
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
     return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
            (Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
             Style.BraceWrapping.AfterEnum) ||
            (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
            (Line.startsWith(tok::kw_struct) && Style.BraceWrapping.AfterStruct);
   if (Left.is(TT_ObjCBlockLBrace) &&
       Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
     return true;
 
   if (Left.is(TT_LambdaLBrace)) {
     if (IsFunctionArgument(Left) &&
         Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline)
       return false;
 
     if (Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
         Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||
         (!Left.Children.empty() &&
          Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Empty))
       return true;
   }
 
+  if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace) &&
+      Left.isOneOf(tok::star, tok::amp, tok::ampamp)) {
+    return true;
+  }
+
   // Put multiple Java annotation on a new line.
   if ((Style.Language == FormatStyle::LK_Java ||
        Style.Language == FormatStyle::LK_JavaScript) &&
       Left.is(TT_LeadingJavaAnnotation) &&
       Right.isNot(TT_LeadingJavaAnnotation) && Right.isNot(tok::l_paren) &&
       (Line.Last->is(tok::l_brace) || Style.BreakAfterJavaFieldAnnotations))
     return true;
 
if ((Left.isBinaryOperator() || Left.is(TT_BinaryOperator)) &&
       (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
        Left.getPrecedence() == prec::Assignment))
     return true;
   if ((Left.is(TT_AttributeSquare) && Right.is(tok::l_square)) ||
       (Left.is(tok::r_square) && Right.is(TT_AttributeSquare)))
     return false;
 
   auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
-  if (Style.BraceWrapping.BeforeLambdaBody) {
+  if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) {
     if (isAllmanLambdaBrace(Left))
       return !isItAnEmptyLambdaAllowed(Left, ShortLambdaOption);
     if (isAllmanLambdaBrace(Right))
       return !isItAnEmptyLambdaAllowed(Right, ShortLambdaOption);
   }
 
   return Left.isOneOf(tok::comma, tok::coloncolon, tok::semi, tok::l_brace,
                       tok::kw_class, tok::kw_struct, tok::comment) ||
          Right.isMemberAccess() ||
          Right.isOneOf(TT_TrailingReturnArrow, TT_LambdaArrow, tok::lessless,
                        tok::colon, tok::l_square, tok::at) ||
-         (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) ||
          (Left.is(tok::r_paren) &&
           Right.isOneOf(tok::identifier, tok::kw_const)) ||
          (Left.is(tok::l_paren) && !Right.is(tok::r_paren)) ||
          (Left.is(TT_TemplateOpener) && !Right.is(TT_TemplateCloser));
 }
 
 void TokenAnnotator::printDebugInfo(const AnnotatedLine &Line) {
   llvm::errs() << "AnnotatedTokens(L=" << Line.Level << "):\n";

clang/unittests/Format/FormatTest.cpp

verifyFormat("TwoNestedLambdas_SLS_None(\n"
                "    {\n"
                "      return Call(\n"
                "          []()\n"
                "          {\n"
                "            return 17;\n"
                "          });\n"
                "    });",
                LLVMWithBeforeLambdaBody);
-  verifyFormat("void Fct()\n"
-               "{\n"
+  verifyFormat("void Fct() {\n"
                "  return {[]()\n"
                "          {\n"
                "            return 17;\n"
                "          }};\n"
                "}",
                LLVMWithBeforeLambdaBody);
 
   LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
verifyFormat("TwoNestedLambdas_SLS_All(\n"
                "      return Call(\n"
                "          []()\n"
                "          {\n"
                "            // A cool function...\n"
                "            return 17;\n"
                "          });\n"
                "    });",
                LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+      FormatStyle::ShortLambdaStyle::SLS_None;
+
+  verifyFormat("auto select = [this]() -> const Library::Object *\n"
+               "{\n"
+               "  return MyAssignment::SelectFromList(this);\n"
+               "};\n",
+               LLVMWithBeforeLambdaBody);
+
+  verifyFormat("auto select = [this]() -> const Library::Object &\n"
+               "{\n"
+               "  return MyAssignment::SelectFromList(this);\n"
+               "};\n",
+               LLVMWithBeforeLambdaBody);
+
+  verifyFormat("namespace test {\n"
+               "class Test {\n"
+               "public:\n"
+               "  Test() = default;\n"
+               "};\n"
+               "} // namespace test",
+               LLVMWithBeforeLambdaBody);
 }
 
 TEST_F(FormatTest, LambdaWithLineComments) {
   FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
   LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
   LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
   LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
       FormatStyle::ShortLambdaStyle::SLS_All;