[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357027: [clang-format] Add style option 
AllowShortLambdasOnASingleLine (authored by rdwampler, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57687?vs=191872&id=192337#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/FormatToken.h
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -306,6 +306,39 @@
   /// 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;
@@ -1805,6 +1838,7 @@
R.AllowShortFunctionsOnASingleLine &&
AllowShortIfStatementsOnASingleLine ==
R.AllowShortIfStatementsOnASingleLine &&
+   AllowShortLambdasOnASingleLine == R.AllowShortLambdasOnASingleLine &&
AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
AlwaysBreakAfterReturnType == R.AlwaysBreakAfterReturnType &&
AlwaysBreakBeforeMultilineStrings ==
Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -65,6 +65,7 @@
   TYPE(JsTypeOperator) \
   TYPE(JsTypeOptionalQuestion) \
   TYPE(LambdaArrow)\
+  TYPE(LambdaLBrace)   \
   TYPE(LambdaLSquare)  \
   TYPE(LeadingJavaAnnotation)  \
   TYPE(LineComment)\
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1475,6 +1475,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -119,6 +119,17 @@
   }
 };
 
+template <> struct ScalarEnumerationTraits {
+  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 {
   static void enumeration(IO &IO, FormatStyle::BinPackStyle &Value) {
 IO.enumCase(Value, "Auto", FormatStyle::BPS_Auto);
@@ -347,6 +358,8 @@
Style.AllowShortCaseLabelsOnASingleLine);
 IO.mapOptional("AllowShortFunctionsOnASingleLine",
Style.AllowShortFunctionsOnASingleLine);
+IO.mapOptional("AllowShortLambdasOnASingleLine",
+   S

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Manuel Klimek via Phabricator via cfe-commits]

klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

LG


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 191872.
rdwampler added a comment.

Rebased on master.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12239,6 +12239,43 @@
   "> {\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) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1475,6 +1475,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1191,11 +1191,11 @@
 
 // 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;
@@ -2896,7 +2896,7 @@
 // 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,
@@ -3024,6 +3024,19 @@
   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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2976
+
+return Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
+   Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||

klimek wrote:
> If SLS_All is supposed to not change anything, should we fall through here if 
> (SLS_All)?
Yes, I think that's a better approach to ensure the current behavior is 
unchanged.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 191629.
rdwampler marked 2 inline comments as done.
rdwampler added a comment.

Changes since last revision:

-rebased 
-Fall through when SLS_All.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12239,6 +12239,43 @@
   "> {\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) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1475,6 +1475,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1145,11 +1145,11 @@
 
 // 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;
@@ -2849,7 +2849,7 @@
 // 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,
@@ -2977,6 +2977,19 @@
   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_Em

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2976
+
+return Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
+   Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

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)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler marked an inline comment as done.
rdwampler added inline comments.



Comment at: clang/lib/Format/Format.cpp:649
   LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
+  LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;

MyDeveloperDay wrote:
> 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?)
AFAICT, the current behavior is to always put lambdas on a single line, if 
possible.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/Format.cpp:649
   LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
+  LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;

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?)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 187260.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11966,6 +11966,43 @@
   "> {\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) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1436,6 +1436,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -365,7 +365,7 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
@@ -1142,11 +1142,11 @@
 
 // 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;
@@ -2839,7 +2839,7 @@
 // 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,
@@ -2967,6 +2967,18 @@
   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)

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 185848.
rdwampler added a comment.

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!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57687/new/

https://reviews.llvm.org/D57687

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11954,6 +11954,43 @@
   "> {\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) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1433,6 +1433,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -365,7 +365,7 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
@@ -1138,11 +1138,11 @@
 
 // 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;
@@ -2835,7 +2835,7 @@
 // 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,
@@ -2963,6 +2963

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler created this revision.
rdwampler added reviewers: djasper, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57687

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11954,6 +11954,30 @@
   "> {\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);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1433,6 +1433,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -365,7 +365,7 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
@@ -1138,11 +1138,11 @@
 
 // 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;
@@ -2835,7 +2835,7 @@
 // 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,
@@ -2962,6 +2962,10 @@
(Line.startsWith(t