[clang] [clang-format] Fix erroneous BraceWrapping.BeforeLambdaBody column calcs (PR #76673)
James Grant via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 1 15:42:05 PST 2024
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-nz at 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 102504182c4505..f4f8b694f7ff51 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 881993ede17c3d..f5aadec3500ccb 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-nz at 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 f5aadec3500ccb..75557c3cbfaaf8 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);",
More information about the cfe-commits
mailing list