[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-07-03 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:627
 }
+if(Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare)) {
+++Left->BlockParameterCount;

Wawha wrote:
> klimek wrote:
> > Why do we want to increase the parameter count here? Specifically, why is 
> > it not enough to || this condition to the first is in the function?
> I'm not sure to understand. You want to move this test in the first "if" of 
> updateParameterCount()?
> Like :
> 
> ```
> if ((Current->is(tok::l_brace) && Current->BlockKind == BK_Block) ||
>  Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare))
> ++Left->BlockParameterCount;
> ```
> If it's the case, doing that won't work, because updateParameterCount() is 
> call only for brace, not square, so the condition while be true only once and 
> the BlockParameterCount while be equal to 1.
> And ContinuationIndenter::moveStatePastScopeOpener() while require a value 
> greater than 1 to flag true 'HasMultipleNestedBlocks'.
> Same for ParameterCount.
> 
> I had this code because I was thinking that you want me to have this 
> BlockParameterCount equal to 2 in case a lambda is alone inside a function.
Ok, sorry, I'll need to look at this on more detail again. If we only have one 
nested block Blockparametercount should probably be one. I'm unfortunately out 
for the next two weeks. If Sam or Daniel or somebody else can look at that it 
might work earlier, otherwise we'll need to wait. 


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-07-03 Thread Francois JEAN via Phabricator via cfe-commits
Wawha added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:627
 }
+if(Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare)) {
+++Left->BlockParameterCount;

klimek wrote:
> Why do we want to increase the parameter count here? Specifically, why is it 
> not enough to || this condition to the first is in the function?
I'm not sure to understand. You want to move this test in the first "if" of 
updateParameterCount()?
Like :

```
if ((Current->is(tok::l_brace) && Current->BlockKind == BK_Block) ||
 Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare))
++Left->BlockParameterCount;
```
If it's the case, doing that won't work, because updateParameterCount() is call 
only for brace, not square, so the condition while be true only once and the 
BlockParameterCount while be equal to 1.
And ContinuationIndenter::moveStatePastScopeOpener() while require a value 
greater than 1 to flag true 'HasMultipleNestedBlocks'.
Same for ParameterCount.

I had this code because I was thinking that you want me to have this 
BlockParameterCount equal to 2 in case a lambda is alone inside a function.


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-07-03 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added inline comments.
Herald added a subscriber: acoomans.



Comment at: lib/Format/TokenAnnotator.cpp:627
 }
+if(Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare)) {
+++Left->BlockParameterCount;

Why do we want to increase the parameter count here? Specifically, why is it 
not enough to || this condition to the first is in the function?


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-07-02 Thread Francois JEAN via Phabricator via cfe-commits
Wawha updated this revision to Diff 153813.
Wawha added a comment.

Here the third version to manage break with lambda in Allman.
I implement the modification propose by klimek.
If a lambda is detected, the BlockParameterCount and ParameterCount are 
increment one more time in order to avoid extra line parsing after. It's lead 
to less code and less computation time, so I prefer this modification too.
Thank to klimek for this great remark :).

I hope that the patch is now ok for a submission. If there is other needs or 
remarks, tell me.


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,49 @@
   "> {\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"
+   "},\n"
+   "87);",
+   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
@@ -624,6 +624,10 @@
 } else if (Left->ParameterCount == 0 && Current->isNot(tok::comment)) {
   Left->ParameterCount = 1;
 }
+if(Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare)) {
+++Left->BlockParameterCount;
+++Left->ParameterCount;
+}
   }
 
   bool parseConditional() {
@@ -1108,11 +1112,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 = T

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-06-27 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment.

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

>   Hello,
>
> after my last modification (require by previous comment), I do not see any 
> feedback or validation for this patch.
>  Is their something special to do in order to go forward on this patch?
>
> Lambda are more an more used in modern C++, and it's very annoying to not 
> have a way to format them in allman style.


Sorry for the delay, pinging it once a week until somebody replies is the right 
approach.




Comment at: lib/Format/ContinuationIndenter.cpp:1308
+   (Style.BraceWrapping.BeforeLambdaBody &&
+State.Line->containsAfter(Current, TT_LambdaLSquare)));
   State.Stack.back().IsInsideObjCArrayLiteral =

Why don't lambdas already count as nested blocks? Generally, I'd rather fix 
BlockParameterCount than introduce another linear scan over the line.


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-06-26 Thread Francois JEAN via Phabricator via cfe-commits
Wawha marked 2 inline comments as done.
Wawha added a comment.

  Hello,

after my last modification (require by previous comment), I do not see any 
feedback or validation for this patch.
Is their something special to do in order to go forward on this patch?

Lambda are more an more used in modern C++, and it's very annoying to not have 
a way to format them 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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-06-03 Thread mephi42 via Phabricator via cfe-commits
mephi42 added a comment.

This change is very useful for me (I even rebuilt my clang-format with it), but 
there were no updates for quite some time - do you still intend to integrate it?

Apologies in advance if Phabricator is not the right place for such discussions.


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-04-28 Thread Francois JEAN via Phabricator via cfe-commits
Wawha updated this revision to Diff 144460.
Wawha added a reviewer: klimek.
Wawha added a comment.

Hi klimek,
I upload a new patch with the modifications you proposed.
The option is now enable by default in Allman style. So I move the option to 
the BraceWrappingFlags struct, which make more sense.

I also make the second modification you propose to avoid parsing the complete 
line.
Tell me if it's fit your remarks.


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/TokenAnnotator.h
  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,49 @@
   "> {\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"
+   "},\n"
+   "87);",
+   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.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -95,6 +95,17 @@
   template  bool endsWith(Ts... Tokens) const {
 return Last && Last->endsSequence(Tokens...);
   }
+  /// \c true if this line, starting at token 'Start', contains the given tokens in order,
+  /// ignoring comments.
+  template  bool containsAfter(const FormatToken& Start, Ts... Tokens) const {
+const FormatToken *Tok = &Start;
+while (Tok) {
+if(Tok->startsSequence(Tokens...))
+return true;
+Tok = Tok->Next;
+}
+return false;
+  }
 
   /// \c true if this line looks like a function definition instead of a
   /// function declaration. Asserts MightBeFunctionDecl.
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_T

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-04-27 Thread Francois JEAN via Phabricator via cfe-commits
Wawha added a comment.

  Hi klimek,

many thank for your comments. I will made the modifications you propose and 
then update this patch.


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-04-26 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added inline comments.



Comment at: include/clang/Format/Format.h:891
+  /// \endcode
+  bool BreakBeforeLambdaBody;
+

I'd just make that default for Allman style.



Comment at: lib/Format/TokenAnnotator.cpp:2844
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
-return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
+return (Line.containsBefore(Right, TT_LambdaLSquare) && 
Style.BreakBeforeLambdaBody) ||
+   (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||

Instead of parsing the whole line again, which can lead to false positives, you 
can add a TokenType for LambdaLBrace while parsing and use that here.


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-04-23 Thread Francois JEAN via Phabricator via cfe-commits
Wawha added a comment.

  Hi djasper,

Here a project where there is lambda and allman style for them : 
https://github.com/boostorg/hof/blob/develop/example/in.cpp
https://github.com/boostorg/hof/blob/develop/example/sequence.cpp

Example of code:

  // Function to find an iterator using a containers built-in find if available
  BOOST_HOF_STATIC_LAMBDA_FUNCTION(find_iterator) = first_of(
  [](const std::string& s, const auto& x)
  {
  auto index = s.find(x);
  if (index == std::string::npos) return s.end();
  else return s.begin() + index;
  },
  #ifdef _MSC_VER
  // On MSVC, trailing decltype doesn't work with generic lambdas, so a
  // seperate function can be used instead.
  BOOST_HOF_LIFT(member_find),
  #else
  [](const auto& r, const auto& x) BOOST_HOF_RETURNS(r.find(x)),
  #endif
  [](const auto& r, const auto& x)
  {
  using std::begin;
  using std::end;
  return std::find(begin(r), end(r), x);
  }
  );

What could be the next to move forward on this topic?
Do you think that some modification should be make on this patch? Change option 
name? Make it default behavior for allman?


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-04-06 Thread Francois JEAN via Phabricator via cfe-commits
Wawha added a comment.

I'm not working on a public project with a public style guide, so not sure it 
will fit all the requirement inside. But perhaps the request of Rian for 
bareflank (https://bugs.llvm.org//show_bug.cgi?id=32151#c4) could help to fit 
the needs.

The current patch do not exactly do what he ask, but like you say, it's not 
easy to find all options or one options to match lambda formatting.

In my case, the main requirement is to be able **avoid** a lambda in one line. 
It's not easy to manage, when you put a breakpoint on a such line. It could 
break when the function taking the lambda in arg is call, or when the lambda 
body is executed. Which very annoying for debugging.
As example:

  return something([] ()  { somethingelse(); }); // Breakpoint one this line 
will break when calling something() AND somethingelse()

For the moment, there is no way to format is code in another way. We could have 
this possibilities:

  return something(
   [] ()
   {
 somethingelse();
   });

or

  return something([] ()  {
 somethingelse();
   });

or

  return something(
   [] ()  {
 somethingelse();
   });

The current patch are able to manage the first case, and I'm agree it make 
sense in allman option. So I'm ok to have it by default for allman.


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-04-05 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment.

Please read:

  
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options

In this case in particular, I would be very interested in a style guide that 
defines how Allman brace style and lambdas work together. IMO, it has so many 
corner cases that it mostly doesn't work there. If we find a good and 
consistent way to do this (and your proposal to always wrap before the [] isn't 
a bad idea there), then I think we should make that the default behavior for 
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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-03-28 Thread Francois JEAN via Phabricator via cfe-commits
Wawha added a comment.

I do not know which reviewer to add for this review. Is it possible to give the 
name of the person that will be able to review this code ?


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-03-18 Thread Francois JEAN via Phabricator via cfe-commits
Wawha added a comment.

In https://reviews.llvm.org/D44609#1041260, @lebedev.ri wrote:

> Also, tests?


Sorry, the test files was missing with the first patch. I make mistake using 
svn... I create the new patch with git, it's easier for me.
There is 4 small tests inside unittests/Format/FormatTest.cpp. The first 3 are 
testing the new options, and the last one is here to check that there is no 
regression with initialiser list which contain lambda.


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-03-18 Thread Francois JEAN via Phabricator via cfe-commits
Wawha updated this revision to Diff 138859.
Wawha added a comment.

I upload the full context for these files.


Repository:
  rC Clang

https://reviews.llvm.org/D44609

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11357,6 +11357,42 @@
   "> {\n"
   "  //\n"
   "});");
+
+  // Check option "BreakBeforeLambdaBody"
+  FormatStyle LLVMWithBreakBeforeLambdaBody = getLLVMStyle();
+  LLVMWithBreakBeforeLambdaBody.BreakBeforeLambdaBody = true;
+  verifyFormat("FunctionWithOneParam(\n"
+   "[]()\n"
+   "{\n"
+   "  // A cool function...\n"
+   "  return 43;\n"
+   "},\n"
+   "87);",
+   LLVMWithBreakBeforeLambdaBody);
+  verifyFormat("FunctionWithTwoParams(\n"
+   "[]()\n"
+   "{\n"
+   "  // A cool function...\n"
+   "  return 43;\n"
+   "},\n"
+   "87);",
+   LLVMWithBreakBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call(\n"
+   "  []()\n"
+   "  {\n"
+   "return 17;\n"
+   "  });\n"
+   "});",
+   LLVMWithBreakBeforeLambdaBody);
+  verifyFormat("auto array = {[]()\n"
+   "  {\n"
+   "return 43;\n"
+   "  },\n"
+   "  MyFunctor};",
+   LLVMWithBreakBeforeLambdaBody);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -94,6 +94,28 @@
   template  bool endsWith(Ts... Tokens) const {
 return Last && Last->endsSequence(Tokens...);
   }
+  /// \c true if this line, starting at token 'Start', contains the given tokens in reverse order,
+  /// ignoring comments.
+  template  bool containsBefore(const FormatToken& Start, Ts... Tokens) const {
+const FormatToken *Tok = &Start;
+while (Tok) {
+if(Tok->startsSequence(Tokens...))
+return true;
+Tok = Tok->Previous;
+}
+return false;
+  }
+  /// \c true if this line, starting at token 'Start', contains the given tokens in order,
+  /// ignoring comments.
+  template  bool containsAfter(const FormatToken& Start, Ts... Tokens) const {
+const FormatToken *Tok = &Start;
+while (Tok) {
+if(Tok->startsSequence(Tokens...))
+return true;
+Tok = Tok->Next;
+}
+return false;
+  }
 
   /// \c true if this line looks like a function definition instead of a
   /// function declaration. Asserts MightBeFunctionDecl.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2841,7 +2841,8 @@
   if (Right.is(TT_InlineASMBrace))
 return Right.HasUnescapedNewline;
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
-return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
+return (Line.containsBefore(Right, TT_LambdaLSquare) && Style.BreakBeforeLambdaBody) ||
+   (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) ||
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -337,6 +337,7 @@
 IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces);
 IO.mapOptional("BreakBeforeInheritanceComma",
Style.BreakBeforeInheritanceComma);
+IO.mapOptional("BreakBeforeLambdaBody", Style.BreakBeforeLambdaBody);
 IO.mapOptional("BreakBeforeTernaryOperators",
Style.BreakBeforeTernaryOperators);
 
@@ -623,6 +624,7 @@
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
   LLVMStyle.BreakBeforeInheritanceComma = false;
+  LLVMStyle.BreakBeforeLambdaBody = false;
   LLVMStyle.BreakStringLiterals = true;
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
Index: lib/Format/ContinuationIndenter.cpp
===

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-03-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Also, tests?


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-03-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Please always upload all patches with full context (`-U99`)


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 BreakBeforeLambdaBody to manage lambda line break inside function parameter call

2018-03-18 Thread Francois JEAN via Phabricator via cfe-commits
Wawha created this revision.
Herald added subscribers: cfe-commits, klimek.

This is a patch to fix the problem identify by this bug: 
https://bugs.llvm.org/show_bug.cgi?id=27640.

When formatting this code with option "BreakBeforeBraces: Allman", the lambda 
body are not put entirely on new lines if the lambda is inside a function call.
Here an example of the current formatting in "allman":

  connect([]() {
foo();
bar();
  });

We the new option, the formatting is like that:

  connect(
[]()
{
  foo();
  bar();
});

It's my first patch for this project, so if I made some mistake, do not 
hesitate to explain me that is wrong.
I write few test to check that the new option work and check that there is no 
regression.

This patch should also meet the request of this bug/request: 
https://bugs.llvm.org//show_bug.cgi?id=32151


Repository:
  rC Clang

https://reviews.llvm.org/D44609

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

Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -872,6 +872,24 @@
   /// \endcode
   bool BreakBeforeInheritanceComma;
 
+  /// \brief Wrap lambda block inside function parameters list.
+  /// \code
+  ///   true:
+  ///   connect(
+  /// []()
+  /// {
+  ///   foo();
+  ///   bar();
+  /// });
+  ///
+  ///   false:
+  ///   connect([]() {
+  /// foo();
+  /// bar();
+  ///   });
+  /// \endcode
+  bool BreakBeforeLambdaBody;
+
   /// \brief If ``true``, consecutive namespace declarations will be on the same
   /// line. If ``false``, each namespace is declared on a new line.
   /// \code
@@ -1723,6 +1741,7 @@
BreakStringLiterals == R.BreakStringLiterals &&
ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas &&
BreakBeforeInheritanceComma == R.BreakBeforeInheritanceComma &&
+   BreakBeforeLambdaBody == R.BreakBeforeLambdaBody &&
ConstructorInitializerAllOnOneLineOrOnePerLine ==
R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
ConstructorInitializerIndentWidth ==
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -94,6 +94,28 @@
   template  bool endsWith(Ts... Tokens) const {
 return Last && Last->endsSequence(Tokens...);
   }
+  /// \c true if this line, starting at token 'Start', contains the given tokens in reverse order,
+  /// ignoring comments.
+  template  bool containsBefore(const FormatToken& Start, Ts... Tokens) const {
+const FormatToken *Tok = &Start;
+while (Tok) {
+if(Tok->startsSequence(Tokens...))
+return true;
+Tok = Tok->Previous;
+}
+return false;
+  }
+  /// \c true if this line, starting at token 'Start', contains the given tokens in order,
+  /// ignoring comments.
+  template  bool containsAfter(const FormatToken& Start, Ts... Tokens) const {
+const FormatToken *Tok = &Start;
+while (Tok) {
+if(Tok->startsSequence(Tokens...))
+return true;
+Tok = Tok->Next;
+}
+return false;
+  }
 
   /// \c true if this line looks like a function definition instead of a
   /// function declaration. Asserts MightBeFunctionDecl.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2841,7 +2841,8 @@
   if (Right.is(TT_InlineASMBrace))
 return Right.HasUnescapedNewline;
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
-return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
+return (Line.containsBefore(Right, TT_LambdaLSquare) && Style.BreakBeforeLambdaBody) ||
+   (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) ||
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -337,6 +337,7 @@
 IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces);
 IO.mapOptional("BreakBeforeInheritanceComma",
Style.BreakBeforeInheritanceComma);
+IO.mapOptional("BreakBeforeLambdaBody", Style.BreakBeforeLambdaBody);
 IO.mapOptional("BreakBeforeTernaryOperators",
Style.BreakBeforeTernaryOperators);
 
@@ -623,6 +624,7 @@
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
   LLVMStyle.BreakBeforeInheritanc