[clang-format] Add style option AllowShortLambdasOnASingleLine

[rdwampler]

Summary

This option AllowShortLambdasOnASingleLine similar to the other AllowShort* options, but applied to C++ lambdas.

I considered making this dependent on AllowShortFunctionsOnASingleLine, but could not think of how the breaking should be behave when the option is set to Inline and InlineOnly.

I could not find a specific coding style that requires short lambdas to always break on a new line, but I do think it's fairly common practice. Also, I note that the patch: https://reviews.llvm.org/D44609 attempts to add an option to allow formatting lambda using the Allman style braces. Here is a coding style that requires Allman style braces, but allows lambdas to be on single line if they are short. So, maybe this option would be required to handle this situation?

Addresses: https://bugs.llvm.org//show_bug.cgi?id=32151

[rdwampler]

Update to include an option Inline to only put short lambda on a single line if used as an argument. See the following code style guide from catboost: ps://github.com/catboost/catboost/blob/master/CPP_STYLE_GUIDE.md#lambda-functions

I'm not sure if Inline is the best name. I welcome suggestions.

Thanks!

[MyDeveloperDay]

Could you add a line in the docs/ReleaseNotes.rst to say what you are adding

[MyDeveloperDay]

clang/lib/Format/Format.cpp

662

What is the difference between what clang-format does now and using SLS_All, i.e. if your introducing a new style shouldn't the default be to not change the exsiting code?

Without trying this myself I would think this needs to be SLS_None? (am I wrong?)

[rdwampler]

clang/lib/Format/Format.cpp

662

AFAICT, the current behavior is to always put lambdas on a single line, if possible.

[MyDeveloperDay]

It might be a good idea to add the nested lambda tests from D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style) into your review to show how this patch copes with those (and the examples from the comments of that review)

[rdwampler]

Ping?

[klimek]

clang/lib/Format/TokenAnnotator.cpp

3033

If SLS_All is supposed to not change anything, should we fall through here if (SLS_All)?

[rdwampler]

Changes since last revision:

-rebased
-Fall through when SLS_All.

[rdwampler]

clang/lib/Format/TokenAnnotator.cpp

3033

Yes, I think that's a better approach to ensure the current behavior is unchanged.

[rdwampler]

Rebased on master.

[klimek]

LG

[rdwampler]

I don't have commit rights, can someone land this for me or would it be better to try and get commit access?

clang/docs/ClangFormatStyleOptions.rst

* ``SIS_Always`` (in configuration: ``Always``)
 
     .. code-block:: c++
 
        if (a) return;
        else {
           return;
        }
 
+**AllowShortLambdasOnASingleLine** (``ShortLambdaStyle``)
+  Dependent on the value, ``auto lambda []() { return 0; }`` can be put on a
+  single line.
+
+  Possible values:
+
+  * ``SLS_None`` (in configuration: ``None``)
+    Never merge lambdas into a single line.
+
+  * ``SLS_Empty`` (in configuration: ``Empty``)
+    Only merge empty lambdas.
+
+    .. code-block:: c++
+
+      auto lambda = [](int a) {}
+      auto lambda2 = [](int a) {
+          return a;
+      };
+
+  * ``SLS_Inline`` (in configuration: ``Inline``)
+    Merge lambda into a single line if argument of a function.
+
+    .. code-block:: c++
+
+      auto lambda = [](int a) {
+          return a;
+      };
+      sort(a.begin(), a.end(), ()[] { return x < y; })
+
+  * ``SLS_All`` (in configuration: ``All``)
+    Merge all lambdas fitting on a single line.
+
+    .. code-block:: c++
+
+      auto lambda = [](int a) {}
+      auto lambda2 = [](int a) { return a; };
+
+
+
 **AllowShortLoopsOnASingleLine** (``bool``)
   If ``true``, ``while (true) continue;`` can be put on a single
   line.
 
 **AlwaysBreakAfterDefinitionReturnType** (``DefinitionReturnTypeBreakingStyle``)
   The function definition return type breaking style to use.  This
   option is **deprecated** and is retained for backwards compatibility.
 

clang/include/clang/Format/Format.h

enum ShortIfStyle {
     ///   }
     /// \endcode
     SIS_Always,
   };
 
   /// If ``true``, ``if (a) return;`` can be put on a single line.
   ShortIfStyle AllowShortIfStatementsOnASingleLine;
 
+  /// Different styles for merging short lambdas containing at most one
+  /// statement.
+  enum ShortLambdaStyle {
+    /// Never merge lambdas into a single line.
+    SLS_None,
+    /// Only merge empty lambdas.
+    /// \code
+    ///   auto lambda = [](int a) {}
+    ///   auto lambda2 = [](int a) {
+    ///       return a;
+    ///   };
+    /// \endcode
+    SLS_Empty,
+    /// Merge lambda into a single line if argument of a function.
+    /// \code
+    ///   auto lambda = [](int a) {
+    ///       return a;
+    ///   };
+    ///   sort(a.begin(), a.end(), ()[] { return x < y; })
+    /// \endcode
+    SLS_Inline,
+    /// Merge all lambdas fitting on a single line.
+    /// \code
+    ///   auto lambda = [](int a) {}
+    ///   auto lambda2 = [](int a) { return a; };
+    /// \endcode
+    SLS_All,
+  };
+
+  /// Dependent on the value, ``auto lambda []() { return 0; }`` can be put on a
+  /// single line.
+  ShortLambdaStyle AllowShortLambdasOnASingleLine;
+
   /// If ``true``, ``while (true) continue;`` can be put on a single
   /// line.
   bool AllowShortLoopsOnASingleLine;
 
   /// Different ways to break after the function definition return type.
   /// This option is **deprecated** and is retained for backwards compatibility.
   enum DefinitionReturnTypeBreakingStyle {
     /// Break after return type automatically.
return AccessModifierOffset == R.AccessModifierOffset &&
                R.AllowAllParametersOfDeclarationOnNextLine &&
            AllowShortBlocksOnASingleLine == R.AllowShortBlocksOnASingleLine &&
            AllowShortCaseLabelsOnASingleLine ==
                R.AllowShortCaseLabelsOnASingleLine &&
            AllowShortFunctionsOnASingleLine ==
                R.AllowShortFunctionsOnASingleLine &&
            AllowShortIfStatementsOnASingleLine ==
                R.AllowShortIfStatementsOnASingleLine &&
+           AllowShortLambdasOnASingleLine == R.AllowShortLambdasOnASingleLine &&
            AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
            AlwaysBreakAfterReturnType == R.AlwaysBreakAfterReturnType &&
            AlwaysBreakBeforeMultilineStrings ==
                R.AlwaysBreakBeforeMultilineStrings &&
            AlwaysBreakTemplateDeclarations ==
                R.AlwaysBreakTemplateDeclarations &&
            BinPackArguments == R.BinPackArguments &&
            BinPackParameters == R.BinPackParameters &&

clang/lib/Format/Format.cpp

static void enumeration(IO &IO, FormatStyle::ShortIfStyle &Value) {
     IO.enumCase(Value, "WithoutElse", FormatStyle::SIS_WithoutElse);
 
     // For backward compatibility.
     IO.enumCase(Value, "false", FormatStyle::SIS_Never);
     IO.enumCase(Value, "true", FormatStyle::SIS_WithoutElse);
   }
 };
 
+template <> struct ScalarEnumerationTraits<FormatStyle::ShortLambdaStyle> {
+  static void enumeration(IO &IO, FormatStyle::ShortLambdaStyle &Value) {
+    IO.enumCase(Value, "None", FormatStyle::SLS_None);
+    IO.enumCase(Value, "false", FormatStyle::SLS_None);
+    IO.enumCase(Value, "Empty", FormatStyle::SLS_Empty);
+    IO.enumCase(Value, "Inline", FormatStyle::SLS_Inline);
+    IO.enumCase(Value, "All", FormatStyle::SLS_All);
+    IO.enumCase(Value, "true", FormatStyle::SLS_All);
+  }
+};
+
 template <> struct ScalarEnumerationTraits<FormatStyle::BinPackStyle> {
   static void enumeration(IO &IO, FormatStyle::BinPackStyle &Value) {
     IO.enumCase(Value, "Auto", FormatStyle::BPS_Auto);
     IO.enumCase(Value, "Always", FormatStyle::BPS_Always);
     IO.enumCase(Value, "Never", FormatStyle::BPS_Never);
   }
 };
 
static void mapping(IO &IO, FormatStyle &Style) {
     IO.mapOptional("AllowAllParametersOfDeclarationOnNextLine",
                    Style.AllowAllParametersOfDeclarationOnNextLine);
     IO.mapOptional("AllowShortBlocksOnASingleLine",
                    Style.AllowShortBlocksOnASingleLine);
     IO.mapOptional("AllowShortCaseLabelsOnASingleLine",
                    Style.AllowShortCaseLabelsOnASingleLine);
     IO.mapOptional("AllowShortFunctionsOnASingleLine",
                    Style.AllowShortFunctionsOnASingleLine);
+    IO.mapOptional("AllowShortLambdasOnASingleLine",
+                   Style.AllowShortLambdasOnASingleLine);
     IO.mapOptional("AllowShortIfStatementsOnASingleLine",
                    Style.AllowShortIfStatementsOnASingleLine);
     IO.mapOptional("AllowShortLoopsOnASingleLine",
                    Style.AllowShortLoopsOnASingleLine);
     IO.mapOptional("AlwaysBreakAfterDefinitionReturnType",
                    Style.AlwaysBreakAfterDefinitionReturnType);
     IO.mapOptional("AlwaysBreakAfterReturnType",
                    Style.AlwaysBreakAfterReturnType);
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.AlignTrailingComments = true;
   LLVMStyle.AlignConsecutiveAssignments = false;
   LLVMStyle.AlignConsecutiveDeclarations = false;
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   LLVMStyle.AllowShortBlocksOnASingleLine = false;
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
+  LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_MultiLine;
   LLVMStyle.BinPackArguments = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;

clang/lib/Format/FormatToken.h

#define LIST_TOKEN_TYPES                                                       \
   TYPE(JsExponentiationEqual)                                                  \
   TYPE(JsFatArrow)                                                             \
   TYPE(JsNonNullAssertion)                                                     \
   TYPE(JsPrivateIdentifier)                                                    \
   TYPE(JsTypeColon)                                                            \
   TYPE(JsTypeOperator)                                                         \
   TYPE(JsTypeOptionalQuestion)                                                 \
   TYPE(LambdaArrow)                                                            \
+  TYPE(LambdaLBrace)                                                           \
   TYPE(LambdaLSquare)                                                          \
   TYPE(LeadingJavaAnnotation)                                                  \
   TYPE(LineComment)                                                            \
   TYPE(MacroBlockBegin)                                                        \
   TYPE(MacroBlockEnd)                                                          \
   TYPE(ObjCBlockLBrace)                                                        \
   TYPE(ObjCBlockLParen)                                                        \
   TYPE(ObjCDecl)                                                               \

clang/lib/Format/TokenAnnotator.cpp

private:
   }
 
   void resetTokenMetadata(FormatToken *Token) {
     if (!Token)
       return;
 
     // Reset token type in case we have already looked at it and then
     // recovered from an error (e.g. failure to find the matching >).
-    if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-                               TT_FunctionLBrace, TT_ImplicitStringLiteral,
-                               TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-                               TT_OverloadedOperator, TT_RegexLiteral,
-                               TT_TemplateString, TT_ObjCStringLiteral))
+    if (!CurrentToken->isOneOf(
+            TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+            TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+            TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+            TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
       CurrentToken->Type = TT_Unknown;
     CurrentToken->Role.reset();
     CurrentToken->MatchingParen = nullptr;
     CurrentToken->FakeLParens.clear();
     CurrentToken->FakeRParens = 0;
   }
 
   void next() {
bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
   if (Left.is(TT_RegexLiteral))
     return false;
   return spaceRequiredBetween(Line, Left, Right);
 }
 
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken &Tok) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
-         !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+         !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
                                      const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
     return true;
 
if (isAllmanBrace(Left) || isAllmanBrace(Right))
     return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
            (Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
             Style.BraceWrapping.AfterEnum) ||
            (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
            (Line.startsWith(tok::kw_struct) && Style.BraceWrapping.AfterStruct);
   if (Left.is(TT_ObjCBlockLBrace) && !Style.AllowShortBlocksOnASingleLine)
     return true;
 
+  if (Left.is(TT_LambdaLBrace)) {
+    if (Left.MatchingParen && Left.MatchingParen->Next &&
+        Left.MatchingParen->Next->isOneOf(tok::comma, tok::r_paren) &&
+        Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline)
+      return false;
+
+    if (Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
+        Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||
+        (!Left.Children.empty() &&
+         Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Empty))
+      return true;
+  }
+
   // Put multiple C# attributes on a new line.
   if (Style.isCSharp() &&
       ((Left.is(TT_AttributeSquare) && Left.is(tok::r_square)) ||
        (Left.is(tok::r_square) && Right.is(TT_AttributeSquare) &&
         Right.is(tok::l_square))))
     return true;
 
   // Put multiple Java annotation on a new line.

clang/lib/Format/UnwrappedLineParser.cpp

case tok::arrow:
       FormatTok->Type = TT_LambdaArrow;
       SeenArrow = true;
       nextToken();
       break;
     default:
       return true;
     }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
 }
 
 bool UnwrappedLineParser::tryToParseLambdaIntroducer() {
   const FormatToken *Previous = FormatTok->Previous;
   if (Previous &&

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, FormatsLambdas) {
   // Lambdas with complex multiline introducers.
   verifyFormat(
       "aaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
       "    [aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]()\n"
       "        -> ::std::unordered_set<\n"
       "            aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa> {\n"
       "      //\n"
       "    });");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+               "  return b;\n"
+               "};",
+               "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+               "};",
+               " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+               "  return b;\n"
+               "};",
+               "auto c = []() {\n"
+               "  return b;\n"
+               " };",
+               MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+               "auto c = []() {\n"
+               "};",
+               MergeEmptyOnly);
+
+  FormatStyle MergeInline = getLLVMStyle();
+  MergeInline.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Inline;
+  verifyFormat("auto c = []() {\n"
+               "  return b;\n"
+               "};",
+               "auto c = []() { return b; };", MergeInline);
+  verifyFormat("function([]() { return b; })", "function([]() { return b; })",
+               MergeInline);
+  verifyFormat("function([]() { return b; }, a)",
+               "function([]() { return b; }, a)", MergeInline);
+  verifyFormat("function(a, []() { return b; })",
+               "function(a, []() { return b; })", MergeInline);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
   verifyFormat("auto lambda = []() {\n"
                "  x(); //\n"
                "};",
                "auto lambda = []() {\n"
                "\n"