[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 04:03:11 PST 2024


https://github.com/jamesg-nz created https://github.com/llvm/llvm-project/pull/76673

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.

>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] [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;



More information about the cfe-commits mailing list