Author: mydeveloperday Date: 2021-06-26T13:34:07+01:00 New Revision: 37c2233097ac44697b87228d86eef1fce10ea5c1
URL: https://github.com/llvm/llvm-project/commit/37c2233097ac44697b87228d86eef1fce10ea5c1 DIFF: https://github.com/llvm/llvm-project/commit/37c2233097ac44697b87228d86eef1fce10ea5c1.diff LOG: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace https://bugs.llvm.org/show_bug.cgi?id=50702 I believe {D44609} 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 Reviewed By: HazardyKnusperkeks Differential Revision: https://reviews.llvm.org/D104222 Added: Modified: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index bfa49de7d74f..5f973950b8d7 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -3579,42 +3579,11 @@ isItAnEmptyLambdaAllowed(const FormatToken &Tok, 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; @@ -3776,13 +3745,6 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line, 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) && @@ -3805,6 +3767,11 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line, return true; } + if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace) && + Left.isOneOf(tok::star, tok::amp, tok::ampamp, TT_TemplateCloser)) { + return true; + } + // Put multiple Java annotation on a new line. if ((Style.Language == FormatStyle::LK_Java || Style.Language == FormatStyle::LK_JavaScript) && @@ -4209,7 +4176,7 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line, 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)) @@ -4221,7 +4188,6 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line, 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)) || diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index e084d06b2aa6..e5fa17270cdf 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -19596,8 +19596,7 @@ TEST_F(FormatTest, FormatsLambdas) { " });\n" " });", LLVMWithBeforeLambdaBody); - verifyFormat("void Fct()\n" - "{\n" + verifyFormat("void Fct() {\n" " return {[]()\n" " {\n" " return 17;\n" @@ -19802,6 +19801,35 @@ TEST_F(FormatTest, FormatsLambdas) { " });", 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("auto select = [this]() -> std::unique_ptr<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); + // Lambdas with diff erent indentation styles. Style = getLLVMStyleWithColumns(100); EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
