[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

2494

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

2500

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

2494

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

2914

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

2914

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

2914

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

2520

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

clang/docs/ClangFormatStyleOptions.rst

**KeepEmptyLinesAtTheStartOfBlocks** (``bool``)
   .. code-block:: c++
 
      true:                                  false:
      if (foo) {                     vs.     if (foo) {
                                               bar();
        bar();                               }
      }
 
+**LambdaBodyIndentation** (``LambdaBodyIndentationKind``)
+  The indentation style of lambda bodies. ``Signature`` (the default)
+  causes the lambda body to be indented one additional level relative to
+  the indentation level of the signature. ``OuterScope`` forces the lambda
+  body to be indented one additional level relative to the parent scope
+  containing the lambda signature. For callback-heavy code, it may improve
+  readability to have the signature indented two levels and to use
+  ``OuterScope``. The KJ style guide requires ``OuterScope`.
+  `KJ style guide
+  <https://github.com/capnproto/capnproto/blob/master/kjdoc/style-guide.md>`_
+
+  Possible values:
+
+  * ``LBI_Signature`` (in configuration: ``Signature``)
+    Align lambda body relative to the lambda signature. This is the default.
+
+    .. code-block:: c++
+
+       someMethod(
+           [](SomeReallyLongLambdaSignatureArgument foo) {
+             return;
+           });
+
+  * ``LBI_OuterScope`` (in configuration: ``OuterScope``)
+    Align lambda body relative to the indentation level of the outer scope
+    the lambda signature resides in.
+
+    .. code-block:: c++
+
+       someMethod(
+           [](SomeReallyLongLambdaSignatureArgument foo) {
+         return;
+       });
+
+
+
 **Language** (``LanguageKind``)
   Language, this format style is targeted at.
 
   Possible values:
 
   * ``LK_None`` (in configuration: ``None``)
     Do not use.
 
**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/).
 
 
 
 **MacroBlockBegin** (``std::string``)
   A regular expression matching macros that start a block.
 
   .. code-block:: c++
 
      # With:
      MacroBlockBegin: "^NS_MAP_BEGIN|\

clang/docs/ReleaseNotes.rst

 
 - Option ``AllowShortIfStatementsOnASingleLine`` has been improved, it now
   accepts ``AllIfsAndElse`` value that allows to put "else if" and "else" short
   statements on a single line. (Fixes https://llvm.org/PR50019.)
 
 - Option ``BreakInheritanceList`` gets a new style, ``AfterComma``. It breaks
   only after the commas that separate the base-specifiers.
 
+- Option ``LambdaBodyIndentation`` has been added to control how the body of a
+  lambda is indented. The default ``Signature`` value indents the body one level
+  relative to whatever indentation the signature has. ``OuterScope`` lets you
+  change that so that the lambda body is indented one level relative to the scope
+  containing the lambda, regardless of where the lambda signature was placed.
+
 - ``git-clang-format`` no longer formats changes to symbolic links. (Fixes
   https://llvm.org/PR46992.)
 
 - Makes ``PointerAligment: Right`` working with ``AlignConsecutiveDeclarations``.
   (Fixes https://llvm.org/PR27353)
 
 - Option ``AlignArrayOfStructure`` has been added to allow for ordering array-like
   initializers.

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;
 
+  /// Indentation logic for lambda bodies.
+  enum LambdaBodyIndentationKind : unsigned char {
+    /// Align lambda body relative to the lambda signature. This is the default.
+    /// \code
+    ///    someMethod(
+    ///        [](SomeReallyLongLambdaSignatureArgument foo) {
+    ///          return;
+    ///        });
+    /// \endcode
+    LBI_Signature,
+    /// Align lambda body relative to the indentation level of the outer scope
+    /// the lambda signature resides in.
+    /// \code
+    ///    someMethod(
+    ///        [](SomeReallyLongLambdaSignatureArgument foo) {
+    ///      return;
+    ///    });
+    /// \endcode
+    LBI_OuterScope,
+  };
+
+  /// The indentation style of lambda bodies. ``Signature`` (the default)
+  /// causes the lambda body to be indented one additional level relative to
+  /// the indentation level of the signature. ``OuterScope`` forces the lambda
+  /// body to be indented one additional level relative to the parent scope
+  /// containing the lambda signature. For callback-heavy code, it may improve
+  /// readability to have the signature indented two levels and to use
+  /// ``OuterScope``. The KJ style guide requires ``OuterScope`.
+  /// `KJ style guide
+  /// <https://github.com/capnproto/capnproto/blob/master/kjdoc/style-guide.md>`_
+  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$"
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.IndentRequires = false;
   LLVMStyle.IndentWrappedFunctionNames = false;
   LLVMStyle.IndentWidth = 2;
   LLVMStyle.PPIndentWidth = -1;
   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"
+               "    int b = 32;\n"
+               "    return 3;\n"
+               "  }\n"
+               "}",
+               Style);
+  verifyFormat("std::sort(v.begin(), v.end(),\n"
+               "          [](const auto &someLongArgumentName, const auto "
+               "&someOtherLongArgumentName) {\n"
+               "  return someLongArgumentName.someMemberVariable < "
+               "someOtherLongArgumentName.someMemberVariable;\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));
+  // TODO: The current formatting has a minor issue that's not worth fixing
+  // right now whereby the closing brace is indented relative to the signature
+  // instead of being aligned. This only happens with macros.
 }
 
 TEST_F(FormatTest, LambdaWithLineComments) {
   FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
   LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
   LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
   LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
       FormatStyle::ShortLambdaStyle::SLS_All;