https://github.com/jamesg-nz updated https://github.com/llvm/llvm-project/pull/76673
>From 04885844162b5390d8041a44a1895ad6ac160228 Mon Sep 17 00:00:00 2001 From: James Grant <42079499+jamesg...@users.noreply.github.com> Date: Mon, 1 Jan 2024 20:27:41 +1300 Subject: [PATCH 1/2] [clang-format] Fix erroneous BraceWrapping.BeforeLambdaBody column calcs Firstly, must check ColumnLimit > 0 before comparing calculated columns to the limit. Otherwise it always breaks before the brace if ColumnLimit = 0 (no limit). Fixes #50275 Secondly, the lambda body length alone is currently compared with the column limit. Should instead be comparing a column position, which includes everything before the lambda body, the body length itself, and any unbreakable tail. Fixes #59724 Thirdly, if must break before the lambda right brace, e.g. line comment in body, then must also break before the left brace. Can't use column calculation in this instance. --- clang/lib/Format/ContinuationIndenter.cpp | 10 ++- clang/unittests/Format/FormatTest.cpp | 78 +++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 102504182c4505b..f4f8b694f7ff51e 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -366,8 +366,14 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { const auto &CurrentState = State.Stack.back(); if (Style.BraceWrapping.BeforeLambdaBody && Current.CanBreakBefore && Current.is(TT_LambdaLBrace) && Previous.isNot(TT_LineComment)) { - auto LambdaBodyLength = getLengthToMatchingParen(Current, State.Stack); - return LambdaBodyLength > getColumnLimit(State); + if (Current.MatchingParen->MustBreakBefore) + return true; + + auto LambdaEnd = getLengthToMatchingParen(Current, State.Stack) + + Current.MatchingParen->UnbreakableTailLength + + State.Column - 1; + + return Style.ColumnLimit > 0 && LambdaEnd > getColumnLimit(State); } if (Current.MustBreakBefore || (Current.is(TT_InlineASMColon) && diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 881993ede17c3dc..f5aadec3500ccb7 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -22965,6 +22965,84 @@ TEST_F(FormatTest, EmptyLinesInLambdas) { "};"); } +TEST_F(FormatTest, BreakBeforeLambdaBodyWrapping) { + verifyFormat("connect([]() {\n" + " foo();\n" + " bar();\n" + "});"); + + auto Style = getLLVMStyle(); + Style.BreakBeforeBraces = FormatStyle::BS_Custom; + Style.BraceWrapping.BeforeLambdaBody = true; + + verifyFormat("connect(\n" + " []()\n" + " {\n" + " foo();\n" + " bar();\n" + " });", + Style); + + for (unsigned l : {0, 41}) { + Style.ColumnLimit = l; + verifyFormat("auto lambda = []() { return foo + bar; };", Style); + } + for (unsigned l : {40, 22}) { + Style.ColumnLimit = l; + verifyFormat("auto lambda = []()\n" + "{ return foo + bar; };", + Style); + } + Style.ColumnLimit = 21; + verifyFormat("auto lambda = []()\n" + "{\n" + " return foo + bar;\n" + "};", + Style); + + for (unsigned l : {0, 67}) { + Style.ColumnLimit = l; + verifyFormat( + "auto result = [](int foo, int bar) { return foo + bar; }(foo, bar);", + Style); + } + Style.ColumnLimit = 66; + verifyFormat("auto result = [](int foo, int bar)\n" + "{ return foo + bar; }(foo, bar);", + Style); + + Style.ColumnLimit = 36; + verifyFormat("myFunc([&]() { return foo + bar; });", Style); + Style.ColumnLimit = 35; + verifyFormat("myFunc([&]()\n" + " { return foo + bar; });", + Style); + + Style = getGoogleStyleWithColumns(100); + Style.BreakBeforeBraces = FormatStyle::BS_Allman; + Style.IndentWidth = 4; + verifyFormat( + "void Func()\n" + "{\n" + " []()\n" + " {\n" + " return TestVeryLongThingName::TestVeryLongFunctionName()\n" + " .TestAnotherVeryVeryLongFunctionName();\n" + " }\n" + "}\n", + Style); + verifyFormat( + "void Func()\n" + "{\n" + " []()\n" + " {\n" + " return TestVeryLongThingName::TestVeryLongFunctionName()\n" + " .TestAnotherVeryVeryVeryLongFunctionName();\n" + " }\n" + "}\n", + Style); +} + TEST_F(FormatTest, FormatsBlocks) { FormatStyle ShortBlocks = getLLVMStyle(); ShortBlocks.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always; >From 346e7da1f6eb184f7fc5212af94f5e7c83d5a494 Mon Sep 17 00:00:00 2001 From: James Grant <42079499+jamesg...@users.noreply.github.com> Date: Tue, 2 Jan 2024 12:41:48 +1300 Subject: [PATCH 2/2] Per feedback, remove loops from test. Have one separate check for ColumnLimit = 0 case. Unroll one loop (that doesn't use 0) into two verifications. --- clang/unittests/Format/FormatTest.cpp | 33 ++++++++++++++------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index f5aadec3500ccb7..75557c3cbfaaf83 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -22983,16 +22983,19 @@ TEST_F(FormatTest, BreakBeforeLambdaBodyWrapping) { " });", Style); - for (unsigned l : {0, 41}) { - Style.ColumnLimit = l; - verifyFormat("auto lambda = []() { return foo + bar; };", Style); - } - for (unsigned l : {40, 22}) { - Style.ColumnLimit = l; - verifyFormat("auto lambda = []()\n" - "{ return foo + bar; };", - Style); - } + Style.ColumnLimit = 0; + verifyFormat("auto noLimit = []() { return 0; };", Style); + + Style.ColumnLimit = 41; + verifyFormat("auto lambda = []() { return foo + bar; };", Style); + Style.ColumnLimit = 40; + verifyFormat("auto lambda = []()\n" + "{ return foo + bar; };", + Style); + Style.ColumnLimit = 22; + verifyFormat("auto lambda = []()\n" + "{ return foo + bar; };", + Style); Style.ColumnLimit = 21; verifyFormat("auto lambda = []()\n" "{\n" @@ -23000,12 +23003,10 @@ TEST_F(FormatTest, BreakBeforeLambdaBodyWrapping) { "};", Style); - for (unsigned l : {0, 67}) { - Style.ColumnLimit = l; - verifyFormat( - "auto result = [](int foo, int bar) { return foo + bar; }(foo, bar);", - Style); - } + Style.ColumnLimit = 67; + verifyFormat( + "auto result = [](int foo, int bar) { return foo + bar; }(foo, bar);", + Style); Style.ColumnLimit = 66; verifyFormat("auto result = [](int foo, int bar)\n" "{ return foo + bar; }(foo, bar);", _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits