[clang] [clang-format] Don't apply severe penalty if no possible column formats (PR #76675)

via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 1 04:31:26 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: James Grant (jamesg-nz)

<details>
<summary>Changes</summary>

If there are possible column formats, but they weren't selected because they don't fit within remaining characters for the current path then applying severe penalty to induce column layout by selection of a different path seems fair.

But if due to style configuration or what the input code is, there are no possible column formats, different paths aren't going to have column layouts. Seems wrong to apply the severe penalty to induce column layouts if there are none available.

It just causes selection of sub-optimal paths, e.g. get bad formatting when brace initializers are used inside lambda bodies.

Fixes #<!-- -->56350

---
Full diff: https://github.com/llvm/llvm-project/pull/76675.diff


2 Files Affected:

- (modified) clang/lib/Format/FormatToken.cpp (+2-2) 
- (modified) clang/unittests/Format/FormatTest.cpp (+15) 


``````````diff
diff --git a/clang/lib/Format/FormatToken.cpp b/clang/lib/Format/FormatToken.cpp
index 7a2df8c53952f9..b791c5a26bbe3a 100644
--- a/clang/lib/Format/FormatToken.cpp
+++ b/clang/lib/Format/FormatToken.cpp
@@ -113,8 +113,8 @@ unsigned CommaSeparatedList::formatAfterToken(LineState &State,
   if (!State.NextToken || !State.NextToken->Previous)
     return 0;
 
-  if (Formats.size() == 1)
-    return 0; // Handled by formatFromToken
+  if (Formats.size() <= 1)
+    return 0; // Handled by formatFromToken (1) or avoid severe penalty (0).
 
   // Ensure that we start on the opening brace.
   const FormatToken *LBrace =
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 881993ede17c3d..b7350b2fe66d9b 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -13875,6 +13875,21 @@ TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
                getLLVMStyleWithColumns(35));
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa, {},\n"
                "                               aaaaaaaaaaaaaaaaaaaaaaa);");
+
+  // No possible column formats, don't want the optimal paths penalized.
+  verifyFormat(
+      "waarudo::unit desk = {\n"
+      "    .s = \"desk\", .p = p, .b = [] { return w::r{3, 10} * w::m; }};");
+  verifyFormat("SomeType something1([](const Input &i) -> Output { return "
+               "Output{1, 2}; },\n"
+               "                    [](const Input &i) -> Output { return "
+               "Output{1, 2}; });");
+  FormatStyle NoBinPacking = getLLVMStyle();
+  NoBinPacking.BinPackParameters = false;
+  verifyFormat("waarudo::unit desk = {\n"
+               "    .s = \"desk\", .p = p, .b = [] { return w::r{3, 10, 1, 1, "
+               "1, 1} * w::m; }};",
+               NoBinPacking);
 }
 
 TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/76675


More information about the cfe-commits mailing list