[clang-format] Add new LambdaBodyIndentation option

[• vlovich]

Summary

Currently the lambda body indents relative to where the lambda signature is located. This instead lets the user
choose to align the lambda body relative to the parent scope that contains the lambda declaration. Thus:

someFunction([] {
  lambdaBody();
});

will always have the same indentation of the body even when the lambda signature goes on a new line:

someFunction(
    [] {
  lambdaBody();
});

whereas before lambdaBody would be indented 6 spaces.

[• vlovich]

I'm not 100% certain if the implementation is absolutely correct. For example, the #define A test case is broken (the closing brace is indented incorrectly). Additionally, the pop_back feels weird but seems to fix all the other cases to make the closing brace aligned properly. Maybe some/all of the implementation belongs in ContinuationIndenter.cpp instead but I couldn't really follow where the best place to do that might be. I can also just leave the preprocessor macro case as a known issue for now.

[HazardyKnusperkeks]

Maybe a bit more test cases with smaller lambdas? Or without the outer parenthesis?

clang/include/clang/Format/Format.h

2456

I think you should add examples, because for me it is not 100% what's the difference is.

2462

Here should also a comment be added.

[• vlovich]

In D102706#2766680, @HazardyKnusperkeks wrote:

Maybe a bit more test cases with smaller lambdas? Or without the outer parenthesis?

I'm not sure I understand this comment. Which test case are you referring to by "or without the outer parenthesis"?

clang/include/clang/Format/Format.h

2456

Good idea. Meant to do that & forgot. In terms of an example, do you find the test case or description in the phabricator message useful or are you looking for something else?

[HazardyKnusperkeks]

In D102706#2766871, @vlovich wrote:

In D102706#2766680, @HazardyKnusperkeks wrote:

Maybe a bit more test cases with smaller lambdas? Or without the outer parenthesis?

I'm not sure I understand this comment. Which test case are you referring to by "or without the outer parenthesis"?

verifyFormat("test() {\n"
               "  ([]() -> {\n"
               "    int b = 32;\n"
               "    return 3;\n"
               "  }).foo();\n"
               "}",
               Style);

There you have parenthesis around the lambda, how about without?
Maybe just something like

std::sort(v.begin(), v.end(), [](const auto& lhs, const auto& rhs) { return lhs.Foo < rhs.Foo; });

[• vlovich]

verifyFormat("test() {\n"

"  ([]() -> {\n"
"    int b = 32;\n"
"    return 3;\n"
"  }).foo();\n"
"}",
Style);

There you have parenthesis around the lambda, how about without?
Maybe just something like

std::sort(v.begin(), v.end(), [](const auto& lhs, const auto& rhs) { return lhs.Foo < rhs.Foo; });

Ah. Ok. What I had done was run all the tests with this flag & note the ones where the formatting changed & copied those. The one without parenthesis didn't do anything interesting which is why I omitted it originally. I'll add it back + your sort example too. Thanks!

[• vlovich]

Review response

• Add examples to documentation + links to KJ style guide.
• Add better header doc
• Add more test cases
• Add release notes
• Note current "problem" test with a TODO (corner case that's not critical for KJ).

[MyDeveloperDay]

Is `Signature``` effectively as is?

[MyDeveloperDay]

Could you clang-format the tests please so I can more easily read them.

clang/docs/ClangFormatStyleOptions.rst

2790

Did you generate this by hand or using the clang/doc/tools/dump_style.py?

[• vlovich]

In D102706#2774018, @MyDeveloperDay wrote:

Could you clang-format the tests please so I can more easily read them.

Sure. I was just taking my queue from the rest of the file which appeared to intentionally not clang-format the test strings.

clang/docs/ClangFormatStyleOptions.rst

2790

By hand. dump_format_style generates other changes (although I wasn't aware of it at the time). I'll regenerate just this section using that tool.

[• vlovich]

In D102706#2774010, @MyDeveloperDay wrote:

Is `Signature``` effectively as is?

Yes. I noted that it's the default behavior. Please let me know if I can clarify the documentation wording somehow (or if you have preferred wording you want me to use)

[• vlovich]

Regen documentation & clang-format test cases.

[MyDeveloperDay]

I personally like to keep clang-format and clang-format tests clang formatted all the time so we have a reasonable set of "clang-format-clean" code we can sanity check any change against, the more of LLVM that can be clean the better our testing can be.

clang/docs/ClangFormatStyleOptions.rst

2790

it shouldn't, that probably means someone has made a change without keeping them consistent..I'll take a look at that.

[• vlovich]

I think all review comments have been addressed. Please let me know if there's anything else blocking merge.

[HazardyKnusperkeks]

LGTM, but please wait for more responses.

[HazardyKnusperkeks]

In D102706#2787550, @HazardyKnusperkeks wrote:

LGTM, but please wait for more responses.

No one objected, so I declare it ready to commit. Do you need some one to do it? If yes please post name and email for the commit.

[• vlovich]

I don't think I have permissions. Happy to do it if I'm given permissions (I'm assuming the instructions are the general LLVM ones). Otherwise:

Name: Vitali Lovich
E-mail: [e-mail]

[HazardyKnusperkeks]

In D102706#2823087, @vlovich wrote:

I don't think I have permissions. Happy to do it if I'm given permissions (I'm assuming the instructions are the general LLVM ones). Otherwise:

Name: Vitali Lovich
E-mail: [e-mail]

You can apply for commit access, yes that's for whole of LLVM, but for me it was no problem to get that right.

[• vlovich]

I think you can go ahead & commit it for me. I don't know when I'll get commit access.

[HazardyKnusperkeks]

clang/include/clang/Format/Format.h

2482

Here is a ` missing, I'll fix it.

clang/docs/ClangFormatStyleOptions.rst

**Language** (``LanguageKind``)
 
   * ``LK_TableGen`` (in configuration: ``TableGen``)
     Should be used for TableGen code.
 
   * ``LK_TextProto`` (in configuration: ``TextProto``)
     Should be used for Protocol Buffer messages in text format
     (https://developers.google.com/protocol-buffers/).
 
 
 
+**LambdaBodyIndentation** (``LambdaBodyIndentationKind``)
+  What is the lambda body indented relative to (default is ``Signature``).
+
+  Possible values:
+  * ``LBI_Signature`` (in configuration: ``Signature``)
+    This uses the default indentation relative to the lambda signature.
+
+  * ``LBI_OuterScope`` (in configuration: ``OuterScope``)
+    This indents relative to the indentation level within which the
+    lambda is encountered.
+
+
+
 **MacroBlockBegin** (``std::string``)
   A regular expression matching macros that start a block.
 
   .. code-block:: c++
 
      # With:
      MacroBlockBegin: "^NS_MAP_BEGIN|\
      NS_TABLE_HEAD$"

clang/include/clang/Format/Format.h

enum LanguageKind : unsigned char {
     LK_TextProto
   };
   bool isCpp() const { return Language == LK_Cpp || Language == LK_ObjC; }
   bool isCSharp() const { return Language == LK_CSharp; }
 
   /// Language, this format style is targeted at.
   LanguageKind Language;
 
+  enum LambdaBodyIndentationKind : unsigned char {
+    /// Align lambda body relative to the lambda signature. This is the default.
+    LBI_Signature,
+    /// Align lambda body relative to the indentation level the lambda lives at.
+    LBI_OuterScope,
+  };
+
+  LambdaBodyIndentationKind LambdaBodyIndentation;
+
   /// A regular expression matching macros that start a block.
   /// \code
   ///    # With:
   ///    MacroBlockBegin: "^NS_MAP_BEGIN|\
   ///    NS_TABLE_HEAD$"
   ///    MacroBlockEnd: "^\
   ///    NS_MAP_END|\
   ///    NS_TABLE_.*_END$"
   ///
   ///    NS_MAP_BEGIN
   ///      foo();
   ///    NS_MAP_END
   ///
   ///    NS_TABLE_HEAD
   ///      bar();
   ///    NS_TABLE_FOO_END
   ///
   ///    # Without:
   ///    NS_MAP_BEGIN
   ///    foo();
   ///    NS_MAP_END
   ///
   ///    NS_TABLE_HEAD
   ///    bar();
   ///    NS_TABLE_FOO_END
   /// \endcode
   std::string MacroBlockBegin;
return AccessModifierOffset == R.AccessModifierOffset &&
            IndentRequires == R.IndentRequires && IndentWidth == R.IndentWidth &&
            Language == R.Language &&
            IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
            JavaImportGroups == R.JavaImportGroups &&
            JavaScriptQuotes == R.JavaScriptQuotes &&
            JavaScriptWrapImports == R.JavaScriptWrapImports &&
            KeepEmptyLinesAtTheStartOfBlocks ==
                R.KeepEmptyLinesAtTheStartOfBlocks &&
+           LambdaBodyIndentation == R.LambdaBodyIndentation &&
            MacroBlockBegin == R.MacroBlockBegin &&
            MacroBlockEnd == R.MacroBlockEnd &&
            MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&
            NamespaceIndentation == R.NamespaceIndentation &&
            NamespaceMacros == R.NamespaceMacros &&
            ObjCBinPackProtocolList == R.ObjCBinPackProtocolList &&
            ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
            ObjCBreakBeforeNestedBlockParam ==

clang/lib/Format/Format.cpp

static void enumeration(IO &IO, FormatStyle::LanguageStandard &Value) {
     IO.enumCase(Value, "c++20", FormatStyle::LS_Cpp20);
 
     IO.enumCase(Value, "Latest", FormatStyle::LS_Latest);
     IO.enumCase(Value, "Cpp11", FormatStyle::LS_Latest); // Legacy alias
     IO.enumCase(Value, "Auto", FormatStyle::LS_Auto);
   }
 };
 
+template <>
+struct ScalarEnumerationTraits<FormatStyle::LambdaBodyIndentationKind> {
+  static void enumeration(IO &IO,
+                          FormatStyle::LambdaBodyIndentationKind &Value) {
+    IO.enumCase(Value, "Signature", FormatStyle::LBI_Signature);
+    IO.enumCase(Value, "OuterScope", FormatStyle::LBI_OuterScope);
+  }
+};
+
 template <> struct ScalarEnumerationTraits<FormatStyle::UseTabStyle> {
   static void enumeration(IO &IO, FormatStyle::UseTabStyle &Value) {
     IO.enumCase(Value, "Never", FormatStyle::UT_Never);
     IO.enumCase(Value, "false", FormatStyle::UT_Never);
     IO.enumCase(Value, "Always", FormatStyle::UT_Always);
     IO.enumCase(Value, "true", FormatStyle::UT_Always);
     IO.enumCase(Value, "ForIndentation", FormatStyle::UT_ForIndentation);
     IO.enumCase(Value, "ForContinuationAndIndentation",
static void mapping(IO &IO, FormatStyle &Style) {
     IO.mapOptional("IndentWrappedFunctionNames",
                    Style.IndentWrappedFunctionNames);
     IO.mapOptional("InsertTrailingCommas", Style.InsertTrailingCommas);
     IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
     IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
     IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
     IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
                    Style.KeepEmptyLinesAtTheStartOfBlocks);
+    IO.mapOptional("LambdaBodyIndentation", Style.LambdaBodyIndentation);
     IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
     IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
     IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
     IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation);
     IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
     IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
     IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
     IO.mapOptional("ObjCBreakBeforeNestedBlockParam",
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
   LLVMStyle.IndentRequires = false;
   LLVMStyle.IndentWrappedFunctionNames = false;
   LLVMStyle.IndentWidth = 2;
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
   LLVMStyle.TabWidth = 8;
+  LLVMStyle.LambdaBodyIndentation = FormatStyle::LBI_Signature;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
   LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;

clang/lib/Format/UnwrappedLineFormatter.cpp

bool formatChildren(LineState &State, bool NewLine, bool DryRun,
     FormatToken &Previous = *State.NextToken->Previous;
     if (!LBrace || LBrace->isNot(tok::l_brace) || LBrace->isNot(BK_Block) ||
         Previous.Children.size() == 0)
       // The previous token does not open a block. Nothing to do. We don't
       // assert so that we can simply call this function for all tokens.
       return true;
 
     if (NewLine) {
-      int AdditionalIndent = State.Stack.back().Indent -
-                             Previous.Children[0]->Level * Style.IndentWidth;
+      const ParenState &P = State.Stack.back();
+
+      int AdditionalIndent =
+          P.Indent - Previous.Children[0]->Level * Style.IndentWidth;
+
+      if (Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
+          P.NestedBlockIndent == P.LastSpace) {
+        if (State.NextToken->MatchingParen &&
+            State.NextToken->MatchingParen->is(TT_LambdaLBrace)) {
+          State.Stack.pop_back();
+        }
+        if (LBrace->is(TT_LambdaLBrace))
+          AdditionalIndent = 0;
+      }
 
       Penalty +=
           BlockFormatter->format(Previous.Children, DryRun, AdditionalIndent,
                                  /*FixBadIndentation=*/true);
       return true;
     }
 
     if (Previous.Children[0]->First->MustBreakBefore)

clang/unittests/Format/FormatTest.cpp

verifyFormat("llvm::errs() << \"number of twos is \"\n"
                "                  return x == 2; // force break\n"
                "                });");
   verifyFormat("return aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
                "    [=](int iiiiiiiiiiii) {\n"
                "      return aaaaaaaaaaaaaaaaaaaaaaa !=\n"
                "             aaaaaaaaaaaaaaaaaaaaaaa;\n"
                "    });",
                getLLVMStyleWithColumns(60));
+
   verifyFormat("SomeFunction({[&] {\n"
                "                // comment\n"
                "              },\n"
                "              [&] {\n"
                "                // comment\n"
                "              }});");
   verifyFormat("SomeFunction({[&] {\n"
                "  // comment\n"
verifyFormat("TwoNestedLambdas_SLS_All(\n"
                "      return Call(\n"
                "          []()\n"
                "          {\n"
                "            // A cool function...\n"
                "            return 17;\n"
                "          });\n"
                "    });",
                LLVMWithBeforeLambdaBody);
+
+  // Lambdas with different indentation styles.
+  Style = getLLVMStyleWithColumns(100);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+            "  return promise.then(\n"
+            "      [this, &someVariable, someObject = std::mv(s)](std::vector<int> evaluated) mutable {\n"
+            "        return someObject.startAsyncAction().then(\n"
+            "            [this, &someVariable](AsyncActionResult result) mutable { result.processMore(); });\n"
+            "      });\n"
+            "}\n",
+            format("SomeResult doSomething(SomeObject promise) {\n"
+                   "  return promise.then([this, &someVariable, someObject = std::mv(s)](std::vector<int> evaluated) mutable {\n"
+                   "    return someObject.startAsyncAction().then([this, &someVariable](AsyncActionResult result) mutable {\n"
+                   "      result.processMore();\n"
+                   "    });\n"
+                   "  });\n"
+                   "}\n",
+                   Style));
+  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
+  verifyFormat("test() {\n"
+               "  ([]() -> {\n"
+               "    int b = 32;\n"
+               "    return 3;\n"
+               "  }).foo();\n"
+               "}",
+               Style);
+  verifyFormat("test() {\n"
+               "  (\n"
+               "      []() -> {\n"
+               "        int b = 32;\n"
+               "        return 3;\n"
+               "      },\n"
+               "      foo, bar)\n"
+               "      .foo();\n"
+               "}",
+               Style);
+  verifyFormat("test() {\n"
+               "  ([]() -> {\n"
+               "    int b = 32;\n"
+               "    return 3;\n"
+               "  })\n"
+               "      .foo()\n"
+               "      .bar();\n"
+               "}",
+               Style);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+            "  return promise.then(\n"
+            "      [this, &someVariable, someObject = std::mv(s)](std::vector<int> evaluated) mutable {\n"
+            "    return someObject.startAsyncAction().then(\n"
+            "        [this, &someVariable](AsyncActionResult result) mutable { result.processMore(); });\n"
+            "  });\n"
+            "}\n",
+            format("SomeResult doSomething(SomeObject promise) {\n"
+                   "  return promise.then([this, &someVariable, someObject = std::mv(s)](std::vector<int> evaluated) mutable {\n"
+                   "    return someObject.startAsyncAction().then([this, &someVariable](AsyncActionResult result) mutable {\n"
+                   "      result.processMore();\n"
+                   "    });\n"
+                   "  });\n"
+                   "}\n",
+                   Style));
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+            "  return promise.then([this, &someVariable] {\n"
+            "    return someObject.startAsyncAction().then(\n"
+            "        [this, &someVariable](AsyncActionResult result) mutable { result.processMore(); });\n"
+            "  });\n"
+            "}\n",
+            format("SomeResult doSomething(SomeObject promise) {\n"
+                   "  return promise.then([this, &someVariable] {\n"
+                   "    return someObject.startAsyncAction().then([this, &someVariable](AsyncActionResult result) mutable {\n"
+                   "      result.processMore();\n"
+                   "    });\n"
+                   "  });\n"
+                   "}\n",
+                   Style));
+  Style = getGoogleStyle();
+  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
+  EXPECT_EQ("#define A                                       \\\n"
+            "  [] {                                          \\\n"
+            "    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(        \\\n"
+            "        xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx); \\\n"
+            "  }",
+            format("#define A [] { xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx( \\\n"
+                   "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx); }",
+                   Style));
 }
 
 TEST_F(FormatTest, LambdaWithLineComments) {
   FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
   LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
   LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
   LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
       FormatStyle::ShortLambdaStyle::SLS_All;