[clang-format] Stop moving lambda to new line only to indent it more.

[rmarker]

Hanging indents are minimised for lambdas by pushing them onto a new line. However, it could still do this even if it would cause even more hanging indents than it otherwise would.

The handling has been expanded to check for this case and avoid moving the lambda to a new line.

Fix #141575

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: None (rmarker)

Changes

Hanging indents are minimised for lambdas by pushing them onto a new line. However, it could still do this even if it would cause even more hanging indents than it otherwise would.

The handling has been expanded to check for this case and avoid moving the lambda to a new line.

Fix #141575

---

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

2 Files Affected:

• (modified) clang/lib/Format/ContinuationIndenter.cpp (+21-4)
• (modified) clang/unittests/Format/FormatTest.cpp (+14)

diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 4e4e48f90a89f..c6c21c4a5ffcd 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -325,13 +325,30 @@ bool ContinuationIndenter::canBreak(const LineState &State) {
   if (Current.isMemberAccess() && CurrentState.ContainsUnwrappedBuilder)
     return false;
 
-  // Don't create a 'hanging' indent if there are multiple blocks in a single
-  // statement and we are aligning lambda blocks to their signatures.
-  if (Previous.is(tok::l_brace) && State.Stack.size() > 1 &&
+  // Force a lambda onto a new line so that we don't create a 'hanging' indent
+  // if there are multiple blocks in a single statement and we are aligning
+  // lambda blocks to their signatures.
+  if (Previous.is(tok::l_brace) && State.Stack.size() > 2 &&
       State.Stack[State.Stack.size() - 2].NestedBlockInlined &&
       State.Stack[State.Stack.size() - 2].HasMultipleNestedBlocks &&
       Style.LambdaBodyIndentation == FormatStyle::LBI_Signature) {
-    return false;
+    if (!Style.isCpp())
+      return false;
+
+    // Make sure to push lambdas to a new line when they are an argument with
+    // other arguments preceding them.
+    if (State.Stack[State.Stack.size() - 2].StartOfFunctionCall > 0)
+      return false;
+
+    // Only force a new line if it is not just going to create a worse hanging
+    // indent. Otherwise, based on the ContinuationIndentWidth, we could end up
+    // more indented than we would've been. To avoid odd looking breaks, make
+    // sure we save at least IndentWidth.
+    if (State.Stack[State.Stack.size() - 3].Indent +
+            Style.ContinuationIndentWidth + Style.IndentWidth <
+        State.Stack[State.Stack.size() - 2].Indent) {
+      return false;
+    }
   }
 
   // Don't break after very short return types (e.g. "void") as that is often
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index c0633ba3c29b3..e55d82ca82c8b 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -23814,6 +23814,20 @@ TEST_F(FormatTest, FormatsLambdas) {
                "}",
                LLVMWithBeforeLambdaBody);
 
+  // Make sure we don't put the lambda on a new line when it would be indented
+  // more than where it would be otherwise.
+  verifyFormat("if ([]()\n"
+               "    {\n"
+               "      return true;\n"
+               "    }()) {\n"
+               "}",
+               LLVMWithBeforeLambdaBody);
+  verifyFormat("fun([]()\n"
+               "    {\n"
+               "      return 17;\n"
+               "    });",
+               LLVMWithBeforeLambdaBody);
+
   LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
       FormatStyle::ShortLambdaStyle::SLS_Empty;
   verifyFormat("FctWithOneNestedLambdaInline_SLS_Empty(\n"

[HazardyKnusperkeks]

Are you sure it is a good idea to increase the minimum stack size here?

[rmarker]

I think that it is alright, and that in the situations this code targets it should always be true.
The added check below looks further up the stack, so this ensures that it is always valid.

I considered adding separate handling for the case of a smaller stack, but thought that it might just be making the code more complex for no gain.
If a case is found where it does cause a formatting regression, it should be easy enough to fix by adding in such handling.
I.e.

if (Previous.is(tok::l_brace) && State.Stack.size() > 1 &&
      State.Stack[State.Stack.size() - 2].NestedBlockInlined &&
      State.Stack[State.Stack.size() - 2].HasMultipleNestedBlocks &&
      Style.LambdaBodyIndentation == FormatStyle::LBI_Signature) {
    if (State.Stack.size() == 2)
      return false;

[HazardyKnusperkeks]

Please wait for @owenca or @mydeveloperday.