[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Mike Weller via Phabricator via cfe-commits]

Userbla added a comment.

I applied this fix locally to a branch based off llvm 11.x and the 
`FormatTest.FormatsTypedefEnum` test now fails.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3312
+  auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
+  if (Style.BraceWrapping.BeforeLambdaBody &&
+  (isAllmanBraceIncludedBreakableLambda(Left, ShortLambdaOption) ||

actually I think its more likely this that is causing the incorrect breaking..I 
don't see anything in these checks which is validating if the tokens are 
actually part of a LambdaBody or did I miss something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3741
tok::colon, tok::l_square, tok::at) ||
+ (Style.BraceWrapping.BeforeLambdaBody && Right.is(tok::l_brace)) ||
  (Left.is(tok::r_paren) &&

I feel this could be way too aggressive! This is basically saying if I have 
BeforeLamdaBody is true then I will always break before the `{`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

  namespace test {
class Test {
public:
  Test() = default;
};
  } // namespace test

Getting miss formatted when BeforeLambdaBody is true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.
Herald added a subscriber: sstefan1.

Tracking a possible bug, https://bugs.llvm.org/show_bug.cgi?id=50702


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha added a comment.

In D44609#1893390 , 
@christophe-calmejane wrote:

> Nice to finally see this patch integrated!
>
> But, it looks like you didn't include all the test case I posted 1.5y ago in 
> this post, that are still problematic and not formatting correctly with your 
> patch:
>  For example, this simple case do not format correctly:
>
>   paramBeforeAndAfterLambda(8,[](int x){call();},5);
>
>
> The output is:
>
>   paramBeforeAndAfterLambda(
>   8,
>   [](int x)
>   {
>   call();
>   },
>   5);
>  
>
>
> although I would expect to see
>
>   paramBeforeAndAfterLambda(8,
>   [](int x)
>   {
>   call();
>   },
>   5);
>  
>
>
> See my proposed fix in the discussion, but note that I don't think it's clean 
> enough to be accepted :)


Thank for the feedback.

Looking back at your comment and patch, I integrate most of your proposal in 
order to fix the problem with "noexcept" .
I even have a comment of MyDevelopperDay because I reuse the keyword "auto" for 
nextTok variable like it was in your proposal :)

For the bug your are reporting with the line wrap for first parameter, looking 
at the tests I wrote, I see two tests which have should cover that case:

  verifyFormat("FctWithMultipleParams_SLS_Empty(A, B,\n"
  " []()\n"
  " {\n"
  " return 17;\n"
  " });", LLVMWithBeforeLambdaBody);
  
  verifyFormat("TwoNestedLambdas_SLS_Empty(A,\n"
   " []()\n"
   " {\n"
   " return Call([]() {});\n"
   " });", LLVMWithBeforeLambdaBody);

So perhaps there is a mistake, depending of the options or if there is an extra 
parameter after the lambda.
I do not have a PC in the next 10 Days to check that but I Will try have a look 
later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Christophe Calmejane via Phabricator via cfe-commits]

christophe-calmejane added a comment.

In D44609#1875172 , @Wawha wrote:

> In D44609#1871612 , @MyDeveloperDay 
> wrote:
>
> > Correct follow that description on how to request commit access
>
>
> It's done. I have the commit right, and push that change on github : 
> https://github.com/llvm/llvm-project/commit/fa0118e6e588fe303b08e7e06ba28ac1f8d50c68
>
> Thank you for the review and advices!


Nice to finally see this patch integrated!

But, it looks like you didn't include all the test case I posted 1.5y ago in 
this post, that are still problematic and not formatting correctly with your 
patch:
For example, this simple case do not format correctly:

  paramBeforeAndAfterLambda(8,[](int x){call();},5);

The output is:

  paramBeforeAndAfterLambda(
  8,
  [](int x)
  {
  call();
  },
  5);

although I would expect to see

  paramBeforeAndAfterLambda(8,
  [](int x)
  {
  call();
  },
  5);

See my proposed fix in the discussion, but note that I don't think it's clean 
enough to be accepted :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha added a comment.

In D44609#1871612 , @MyDeveloperDay 
wrote:

> Correct follow that description on how to request commit access


It's done. I have the commit right, and push that change on github : 
https://github.com/llvm/llvm-project/commit/fa0118e6e588fe303b08e7e06ba28ac1f8d50c68

Thank you for the review and advices!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfa0118e6e588: [clang-format] Add new option BeforeLambdaBody 
in Allman style. (authored by Wawha).
Herald added a reviewer: jdoerfert.

Changed prior to commit:
  https://reviews.llvm.org/D44609?vs=241875=244523#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12673,6 +12673,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterExternBlock);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeLambdaBody);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyRecord);
@@ -13962,6 +13963,245 @@
"function([]() { return b; }, a)", MergeInline);
   verifyFormat("function(a, []() { return b; })",
"function(a, []() { return b; })", MergeInline);
+
+  // Check option "BraceWrapping.BeforeLambdaBody" and different state of
+  // AllowShortLambdasOnASingleLine
+  FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
+  LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
+  LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_None;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_None = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call(\n"
+   "  []()\n"
+   "  {\n"
+   "return 17;\n"
+   "  });\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("void Fct()\n"
+   "{\n"
+   "  return {[]()\n"
+   "  {\n"
+   "return 17;\n"
+   "  }};\n"
+   "}",
+   LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_Empty;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_Empty([]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmptyInsideAVeryVeryVeryVeryVeryVeryVeryL"
+   "ongFunctionName_SLS_Empty(\n"
+   "[]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithMultipleParams_SLS_Empty(A, B,\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_Empty = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call([]() {});\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(A,\n"
+   "   []()\n"
+   "   {\n"
+   " return Call([]() {});\n"
+   "   });",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat(
+  "FctWithLongLineInLambda_SLS_Empty(\n"
+  "[]()\n"
+  "{\n"
+  "  return 

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Correct follow that description on how to request commit access


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha added a comment.

Thank you @MyDeveloperDay for the validation! I'm happy that my contribution 
(and my work to do it well) is accepted.

And yes, I will be there to help to maintain that code and fix bugs if 
necessary.

About the permission in order to land that patch, I'm not sure that I will need 
to do. On that page, 
https://llvm.org/docs/DeveloperPolicy.html#new-contributors, I see that we can 
contact Chris to have permission on the repo. Is it what you propose? Or is 
there another way to have the permission only on llvm Phabricator?

Once I will know the right way to ask for that permission, I will do it :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

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

This patch is nearly 2 years old, and there has been a lot of discussion and I 
can see you've put a lot of work into adding the tests which prove it works, 
from my perspective this LGTM as long as you are prepared to help resolve any 
issues that might come up as a result of this change.

Please own this change, and if you haven't done so already go get commit 
permission land it. You clearly understand how clang-format works and I feel we 
need more people like yourself, especially if you prepared to fix a bug, and if 
your prepared to keep going at it for 2 years.

Thank you and sorry for the delay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha added a comment.

Hi @MyDeveloperDay
Is the last change ok? That is the next step to be able to validate this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha updated this revision to Diff 241874.
Wawha marked an inline comment as done.
Wawha added a comment.

@MyDeveloperDay
I launch docs/tools/dump_style.py on ClangFormatStyleOptions.rst (it seems to 
generate the same result as the previous diff) and add an entry in the Release 
Note. Tell me if there other documentation to update.

I also removed the "auto" keyword! And rebase with the last version of master 
to check the compilation and UnitTest are still OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12673,6 +12673,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterExternBlock);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeLambdaBody);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyRecord);
@@ -13962,6 +13963,245 @@
"function([]() { return b; }, a)", MergeInline);
   verifyFormat("function(a, []() { return b; })",
"function(a, []() { return b; })", MergeInline);
+
+  // Check option "BraceWrapping.BeforeLambdaBody" and different state of
+  // AllowShortLambdasOnASingleLine
+  FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
+  LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
+  LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_None;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_None = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call(\n"
+   "  []()\n"
+   "  {\n"
+   "return 17;\n"
+   "  });\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("void Fct()\n"
+   "{\n"
+   "  return {[]()\n"
+   "  {\n"
+   "return 17;\n"
+   "  }};\n"
+   "}",
+   LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_Empty;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_Empty([]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmptyInsideAVeryVeryVeryVeryVeryVeryVeryL"
+   "ongFunctionName_SLS_Empty(\n"
+   "[]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithMultipleParams_SLS_Empty(A, B,\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_Empty = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call([]() {});\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(A,\n"
+   "   []()\n"
+   "   {\n"
+   " return Call([]() {});\n"
+   "   });",
+   

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha updated this revision to Diff 241875.
Wawha marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12673,6 +12673,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterExternBlock);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeLambdaBody);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyRecord);
@@ -13962,6 +13963,245 @@
"function([]() { return b; }, a)", MergeInline);
   verifyFormat("function(a, []() { return b; })",
"function(a, []() { return b; })", MergeInline);
+
+  // Check option "BraceWrapping.BeforeLambdaBody" and different state of
+  // AllowShortLambdasOnASingleLine
+  FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
+  LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
+  LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_None;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_None = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call(\n"
+   "  []()\n"
+   "  {\n"
+   "return 17;\n"
+   "  });\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("void Fct()\n"
+   "{\n"
+   "  return {[]()\n"
+   "  {\n"
+   "return 17;\n"
+   "  }};\n"
+   "}",
+   LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_Empty;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_Empty([]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmptyInsideAVeryVeryVeryVeryVeryVeryVeryL"
+   "ongFunctionName_SLS_Empty(\n"
+   "[]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithMultipleParams_SLS_Empty(A, B,\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_Empty = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call([]() {});\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(A,\n"
+   "   []()\n"
+   "   {\n"
+   " return Call([]() {});\n"
+   "   });",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat(
+  "FctWithLongLineInLambda_SLS_Empty(\n"
+  "[]()\n"
+  "{\n"
+  "  return HereAVeryLongLine(ThatWillBeFormatted, OnMultipleLine,\n"
+  "   AndShouldNotBeConsiderAsInline,\n"
+  "   LambdaBodyMustBeBreak);\n"
+  "});",
+  LLVMWithBeforeLambdaBody);
+
+  

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Michael via Phabricator via cfe-commits]

Alundra added a comment.

@Wawha In the same style of this missing feature, I think you are also aware of 
the miss of clang-format about struct/array initializer like: 
https://bugs.llvm.org/show_bug.cgi?id=40411
Basically to have the break for braces after the '=' of the struct/array 
initializer like this example:

  const FMOD_3D_ATTRIBUTES Attributes =
  {
  { WorldPosition.x, WorldPosition.y, WorldPosition.z },
  { 0.0f, 0.0f, 0.0f },
  { 0.0f, 0.0f, 1.0f },
  { 0.0f, 1.0f, 0.0f }};

Actually with the indent configured you end with something like:

  const FMOD_3D_ATTRIBUTES Attributes =
  {
  { WorldPosition.x, WorldPosition.y, WorldPosition.z },
  { 0.0f, 0.0f, 0.0f },
  { 0.0f, 0.0f, 1.0f },
  { 0.0f, 1.0f, 0.0f }};

The only way right now is to have the '{' after the '=' but that breaks your 
coding style:

  const FMOD_3D_ATTRIBUTES Attributes = {
  { WorldPosition.x, WorldPosition.y, WorldPosition.z },
  { 0.0f, 0.0f, 0.0f },
  { 0.0f, 0.0f, 1.0f },
  { 0.0f, 1.0f, 0.0f }};

That looks like on the same topic about braces controls missing feature.
Since you look very familiar, it can be for you a next challenge to add that in 
clang format?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I think if you can fix the missing documentation it looks pretty good to go.

I do like the use of the static functions and the extensive test cases.. thank 
you.




Comment at: clang/lib/Format/ContinuationIndenter.cpp:1442
+// Search for any parameter that is a lambda
+auto const *nextTok = Current.Next;
+while (nextTok != nullptr) {

some people don't like the use of auto if the type isn't obvious,

I think by convention shouldn't this variable be `NextTok`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Please can you regenerate the ClangFormatStyleOptions.rst using 
docs/tools/dump_style.py and add an entry into the clang release notes in the 
clang-format section


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha marked 2 inline comments as done.
Wawha added a comment.

Hello,
@klimek will you have time to review this patch? The current patch integrate 
the last remarks and also modification due to inline lambda.




Comment at: lib/Format/ContinuationIndenter.cpp:1179
+   Current.is(TT_LambdaLSquare;
   }
 

MyDeveloperDay wrote:
> oh boy... I have to say.. that I don't like these kinds of massive if 
> statements without some sort of comment.. as to what it's doing, it's so 
> hard, later on, to read through the various clauses and understand which bit 
> of functionality this section covers
I make some change in the last patch. Hope it's better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha updated this revision to Diff 237563.
Wawha added a comment.

Here a new version of the patch with the last version of the source.
It fixes problem with inline lambda. I add more more UnitTest to test.
I also make few small changes to follow remarks.

Do not hesitate to make remarks, I hope that it will be integrate in the coming 
version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12582,6 +12582,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterExternBlock);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeLambdaBody);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyRecord);
@@ -13865,6 +13866,245 @@
"function([]() { return b; }, a)", MergeInline);
   verifyFormat("function(a, []() { return b; })",
"function(a, []() { return b; })", MergeInline);
+
+  // Check option "BraceWrapping.BeforeLambdaBody" and different state of
+  // AllowShortLambdasOnASingleLine
+  FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
+  LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
+  LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_None;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_None = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call(\n"
+   "  []()\n"
+   "  {\n"
+   "return 17;\n"
+   "  });\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("void Fct()\n"
+   "{\n"
+   "  return {[]()\n"
+   "  {\n"
+   "return 17;\n"
+   "  }};\n"
+   "}",
+   LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_Empty;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_Empty([]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmptyInsideAVeryVeryVeryVeryVeryVeryVeryL"
+   "ongFunctionName_SLS_Empty(\n"
+   "[]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithMultipleParams_SLS_Empty(A, B,\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_Empty = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call([]() {});\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(A,\n"
+   "   []()\n"
+   "   {\n"
+   " return Call([]() {});\n"
+   "   });",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat(
+  "FctWithLongLineInLambda_SLS_Empty(\n"
+  "[]()\n"
+  "{\n"
+  "  return 

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Taw via Phabricator via cfe-commits]

tawmoto added a comment.

In D44609#1798209 , @Alundra wrote:

> Please review and consider a merge of this change which looks to not have 
> news since long time.
>  It's useful for a lot of people and it's awesome that there is a ready or 
> almost ready patchset about it.
>  Thanks a lot!


+1, I need this functionality also


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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Michael via Phabricator via cfe-commits]

Alundra added a comment.

Please review and consider a merge of this change which looks to not have news 
since long time.
It's useful for a lot of people and it's awesome that there is a ready or 
almost ready patchset about it.
Thanks a lot!


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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Thanks for the patch, so sorry its taking a long time, its patches like this 
why I'm about to make a phabricator project for #clang-format alone so that we 
can ensure all clang-format patches can be see in isolation.

I think we need to get this patches rebased to the current trunk and then take 
a final look. if you are still interested, I'll be happy to review it.




Comment at: lib/Format/ContinuationIndenter.cpp:1179
+   Current.is(TT_LambdaLSquare;
   }
 

oh boy... I have to say.. that I don't like these kinds of massive if 
statements without some sort of comment.. as to what it's doing, it's so hard, 
later on, to read through the various clauses and understand which bit of 
functionality this section covers



Comment at: lib/Format/Format.cpp:651
 Expanded.BraceWrapping = {true, true, true, true, true, true, true, true,
   true, true, true, true, true, true, true, true};
 break;

if we are adding new options to BraceWrapping structure wouldn't these 
structures need to change too or wouldn't it potentially become unintialized



Comment at: lib/Format/TokenAnnotator.cpp:2920
+
+// Returns 'true' if 'Tok' is an function argument.
+static bool IsFunctionArgument(const FormatToken ) {

I really like this style of pulling out the clauses into separate functions 
because it helps to describe what its doing without the need for a comment at 
all.


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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Zachary Turner via Phabricator via cfe-commits]

zturner added a reviewer: krasimir.
zturner added a comment.

What's the status of this patch, out of curiosity?  It doesn't seem there were 
any objections to the original idea, just that nobody with ownership over 
clang-format is still actively participating in the review.

+krasimir to maybe shed some light on whether this can move forward.


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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Christian Venegas via Phabricator via cfe-commits]

cvenegas added a comment.

I'm testing this patch on our codebase and it is working pretty well. We use 
the Allman style and the lambda problem has been an issue for many years. One 
thing to note in this patch is that some of the files have CRLF line endings 
but should be LF endings, which is why they're showing so many edits. I'm also 
seeing a clang tidy test failing with this patch. The 
readability-braces-around-statements tests seem to fail because the indent 
width is appending double of what it should.
void test() {

 if (cond("if0") /*comment*/) {
do_something("same-line");
  }

Hope this helps as this patch is the only reason why we still need to build 
clang-format instead of using the prebuilt binaries. Thanks!


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

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha updated this revision to Diff 216061.
Wawha added a comment.
Herald added a subscriber: ormris.

I update the patch for the last version of clang format, especially since 
option "AllowShortLambdasOnASingleLine" has been added (which require more 
change for this patch).
Indeed, for AllowShortLambdasOnASingleLine patch 
(https://reviews.llvm.org/D57687), the isAllmanBrace() function has been 
changed (adding TT_LambdaLBrace) which require more test to handle break for 
lambda body.

Unfortunately, there is one case I'm not able to handle correctly for the 
moment.
With option "BeforeLambdaBody : true", "AllowShortLambdasOnASingleLine : 
SLS_All;" and "BinPackParameters : false", the expected code below:

  FctWithLonglineInLambda(
param,
[]()
{
  return 
HereAVeryLongLineThatWillBeFormattedOnMultipleLineAndShouldNotBeConsiderAsInline;
});

Is formatted like this with the current patch:

  FctWithLonglineInLambda(param, []() {
  return 
HereAVeryLongLineThatWillBeFormattedOnMultipleLineAndShouldNotBeConsiderAsInline;
});

Can someone help me and tell where to look to set break for parameters (and 
lambda) when the whole parameters line (included the lambda body, which is not 
the case actually) do not fit the inside one line.
Before the AllowShortLambdasOnASingleLine option, there is no need for that, 
due to the fact that the lambda was always break. But with this new option, we 
need to handle that too.


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

https://reviews.llvm.org/D44609

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Christophe Calmejane via Phabricator via cfe-commits]

christophe-calmejane added a comment.

I fixed it like this (not sure it's 100% correct though!!)

State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount > 
1;
if (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
Current.Tok.getKind() == tok::TokenKind::l_paren &&
Current.BlockParameterCount >= 1) {
// Search for any parameter that is a lambda
  auto const *nextTok = Current.Next;
  while (nextTok != nullptr) {
if (nextTok->is(TT_LambdaLSquare)) {
  State.Stack.back().HasMultipleNestedBlocks = true;
  break;
}
nextTok = nextTok->Next;
  }
}

It works for all cases I'm using to test:

  noOtherParams([](int x){call();});
  paramBeforeLambda(8,[](int x){call();});
  paramAfterLambda([](int x){call();},5);
  paramBeforeAndAfterLambda(8,[](int x){call();},5);
  doubleLambda(8,[](int x){call();},6,[](float f){return f*2;},5);
  nestedLambda1([](int x){return [](float f){return f*2;};});
  nestedLambda2([](int x){ call([](float f){return f*2;});});
  noExceptCall([](int x) noexcept {call();});
  mutableCall([](int x) mutable{call();});
  
  funcCallFunc(call(),[](int x){call();});
  
  auto const l=[](char v){if(v)call();};
  void f()
  {
return [](char v){ if(v) return v++;};
  }

I still think it's a hack (changing "HasMultipleNestedBlocks"), but it seems to 
work.


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Christophe Calmejane via Phabricator via cfe-commits]

christophe-calmejane added a comment.

Ok I found the issue with noexcept, it's not related to your patch.
There is a missing

  case tok::kw_noexcept:

in

  UnwrappedLineParser::tryToParseLambda


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Christophe Calmejane via Phabricator via cfe-commits]

christophe-calmejane added a comment.

Actually, without your change to HasMultipleNestedBlocks, I'm almost getting 
the expected result: Lambda body is correctly indented (and not by function 
name's length).
The only thing not working (and it's not either way, with or without your 
change to HasMultipleNestedBlocks), is nested lambdas. The body is not properly 
indented (but it's not that bad).

About the "noexcept" issue, it might not be related to your patch, I'm not 
sure. Although the "mutable" tag is working as expected.


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Christophe Calmejane via Phabricator via cfe-commits]

christophe-calmejane added a comment.

Hi,

consider the following code

  myFunc([](int x) {
if (x)
call();
  });

When activating your BeforeLambdaBody option, it changes the code to this 
(breaking the start of the lambda to a new line)

  myFunc(
[](int x)
{
if (x)
call();
});

Another issue if I declare the lambda as noexcept, clang-format changes the 
code to this (looks like it's not detecting and applying your option):

  myFunc([](int x) noexcept {
if (x)
call();
  });


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added a comment.

In https://reviews.llvm.org/D44609#1059675, @Wawha wrote:

> In my case, the main requirement is to be able **avoid** a lambda in one line.


That doesn't work for me.  I would like short lambdas still be formatted in one 
line, when `AllowShortFunctionsOnASingleLine` or a new option is on.


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

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



Comment at: lib/Format/ContinuationIndenter.cpp:1307
+   (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
   State.Stack.back().IsInsideObjCArrayLiteral =

klimek wrote:
> Wawha wrote:
> > klimek wrote:
> > > klimek wrote:
> > > > I think I misunderstood some of this the first time around (and thanks 
> > > > for bearing with me :) - iiuc we want to break for Allman even when 
> > > > there's only one nested block. I think I'll need to take another look 
> > > > when I'm back from vacation, unless Daniel or somebody else finds time 
> > > > before then (will be back in 1.5 weeks)
> > > So, HasMultipleNestedBlocks should only be true if there are multiple 
> > > nested blocks.
> > > What tests break if you remove this change to HasMultipleNestedBlocks?
> > All cases with only one lambda in parameter are break. The Lambda body with 
> > be put on the same line as the function and aligned with [] instead of 
> > putting the body [] on another line with just one simple indentation.
> > 
> > So if restore the line to :
> > 
> > ```
> > State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount > 
> > 1;
> > ```
> > 
> > Here some cases.
> > FormatTest.cpp, ligne 11412:
> > 
> > ```
> > // With my modification
> > FunctionWithOneParam(
> > []()
> > {
> >   // A cool function...
> >   return 43;
> > });
> > 
> > // Without my modification
> > FunctionWithOneParam([]()
> >  {
> >// A cool function...
> >return 43;
> >  });
> > ```
> > The "{" block of the lambda will be aligned on the "[" depending of the 
> > function name length.
> > 
> > 
> > You have the same case with multiple lambdas (FormatTest.cpp, ligne 11433):
> > 
> > ```
> > // With my modification
> > TwoNestedLambdas(
> > []()
> > {
> >   return Call(
> >   []()
> >   {
> > return 17;
> >   });
> > });
> > 
> > // Without my modification
> > TwoNestedLambdas([]()
> >  {
> >return Call([]()
> >{
> >  return 17;
> >});
> >  });
> > ```
> Do we want to always break before lambdas in Allman style?
Perhaps not always. I make that current implementation, because, I have some 
difficulty to make the other. I alreay tried to modified the code to have that :

```
TwoNestedLambdas([]()
{
  return Call(
  []()
  {
return 17;
  });
});
```
With just a tabulation after the line return. But with no success for the 
moment.

I could try again to implement it, and why not, add an option to tell if we 
break or not before the lambda. And set it to false by default.



Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:1307
+   (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
   State.Stack.back().IsInsideObjCArrayLiteral =

Wawha wrote:
> klimek wrote:
> > klimek wrote:
> > > I think I misunderstood some of this the first time around (and thanks 
> > > for bearing with me :) - iiuc we want to break for Allman even when 
> > > there's only one nested block. I think I'll need to take another look 
> > > when I'm back from vacation, unless Daniel or somebody else finds time 
> > > before then (will be back in 1.5 weeks)
> > So, HasMultipleNestedBlocks should only be true if there are multiple 
> > nested blocks.
> > What tests break if you remove this change to HasMultipleNestedBlocks?
> All cases with only one lambda in parameter are break. The Lambda body with 
> be put on the same line as the function and aligned with [] instead of 
> putting the body [] on another line with just one simple indentation.
> 
> So if restore the line to :
> 
> ```
> State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount > 1;
> ```
> 
> Here some cases.
> FormatTest.cpp, ligne 11412:
> 
> ```
> // With my modification
> FunctionWithOneParam(
> []()
> {
>   // A cool function...
>   return 43;
> });
> 
> // Without my modification
> FunctionWithOneParam([]()
>  {
>// A cool function...
>return 43;
>  });
> ```
> The "{" block of the lambda will be aligned on the "[" depending of the 
> function name length.
> 
> 
> You have the same case with multiple lambdas (FormatTest.cpp, ligne 11433):
> 
> ```
> // With my modification
> TwoNestedLambdas(
> []()
> {
>   return Call(
>   []()
>   {
> return 17;
>   });
> });
> 
> // Without my modification
> TwoNestedLambdas([]()
>  {
>return Call([]()
>{
>  return 17;
>});
>  });
> ```
Do we want to always break before lambdas in Allman style?


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha marked 2 inline comments as done.
Wawha added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:1307
+   (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
   State.Stack.back().IsInsideObjCArrayLiteral =

klimek wrote:
> klimek wrote:
> > I think I misunderstood some of this the first time around (and thanks for 
> > bearing with me :) - iiuc we want to break for Allman even when there's 
> > only one nested block. I think I'll need to take another look when I'm back 
> > from vacation, unless Daniel or somebody else finds time before then (will 
> > be back in 1.5 weeks)
> So, HasMultipleNestedBlocks should only be true if there are multiple nested 
> blocks.
> What tests break if you remove this change to HasMultipleNestedBlocks?
All cases with only one lambda in parameter are break. The Lambda body with be 
put on the same line as the function and aligned with [] instead of putting the 
body [] on another line with just one simple indentation.

So if restore the line to :

```
State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount > 1;
```

Here some cases.
FormatTest.cpp, ligne 11412:

```
// With my modification
FunctionWithOneParam(
[]()
{
  // A cool function...
  return 43;
});

// Without my modification
FunctionWithOneParam([]()
 {
   // A cool function...
   return 43;
 });
```
The "{" block of the lambda will be aligned on the "[" depending of the 
function name length.


You have the same case with multiple lambdas (FormatTest.cpp, ligne 11433):

```
// With my modification
TwoNestedLambdas(
[]()
{
  return Call(
  []()
  {
return 17;
  });
});

// Without my modification
TwoNestedLambdas([]()
 {
   return Call([]()
   {
 return 17;
   });
 });
```


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:1307
+   (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
   State.Stack.back().IsInsideObjCArrayLiteral =

klimek wrote:
> I think I misunderstood some of this the first time around (and thanks for 
> bearing with me :) - iiuc we want to break for Allman even when there's only 
> one nested block. I think I'll need to take another look when I'm back from 
> vacation, unless Daniel or somebody else finds time before then (will be back 
> in 1.5 weeks)
So, HasMultipleNestedBlocks should only be true if there are multiple nested 
blocks.
What tests break if you remove this change to HasMultipleNestedBlocks?


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha added a comment.

  Hi klimek,

do you have time to take a look again to this patch?
Is my last patch ok for you?

best regards,
François


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:1307
+   (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
   State.Stack.back().IsInsideObjCArrayLiteral =

I think I misunderstood some of this the first time around (and thanks for 
bearing with me :) - iiuc we want to break for Allman even when there's only 
one nested block. I think I'll need to take another look when I'm back from 
vacation, unless Daniel or somebody else finds time before then (will be back 
in 1.5 weeks)


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Francois JEAN via Phabricator via cfe-commits]

Wawha updated this revision to Diff 155254.
Wawha retitled this revision from "[Clang-Format] New option 
BreakBeforeLambdaBody to manage lambda line break inside function parameter 
call" to "[Clang-Format] New option BeforeLambdaBody to manage lambda line 
break inside function parameter call (in Allman style)".
Wawha added a comment.

Here a new patch, which is very similar to the second patch. I just remove the 
extra line parsing which was no necessary since a new tag TT_LambdaLSquare was 
added.
I'm agree with klimek, my third patch was quite weird and after some test 
contain an error in the code and unittest (inside the first test I added...)

This 4th version is far better while it's very similar to the 2nd patch.


Repository:
  rC Clang

https://reviews.llvm.org/D44609

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -10461,6 +10461,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterExternBlock);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeLambdaBody);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyRecord);
@@ -11403,6 +11404,48 @@
   "> {\n"
   "  //\n"
   "});");
+
+  // Check option "BraceWrapping.BeforeLambdaBody"
+  FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
+  LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
+  LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
+  verifyFormat("FunctionWithOneParam(\n"
+   "[]()\n"
+   "{\n"
+   "  // A cool function...\n"
+   "  return 43;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FunctionWithTwoParams(\n"
+   "[]()\n"
+   "{\n"
+   "  // A cool function...\n"
+   "  return 43;\n"
+   "},\n"
+   "87);",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FunctionWithOneNestedLambdas(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call(\n"
+   "  []()\n"
+   "  {\n"
+   "return 17;\n"
+   "  });\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto array = {[]()\n"
+   "  {\n"
+   "return 43;\n"
+   "  },\n"
+   "  MyFunctor};",
+   LLVMWithBeforeLambdaBody);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1404,6 +1404,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1108,11 +1108,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,9 @@
   if (Right.is(TT_InlineASMBrace))
 return