oleg.smolsky added inline comments.

================
Comment at: unittests/Format/FormatTest.cpp:11604
+               "      x.end(),   //\n"
+               "      [&](int, int) { return 1; });\n"
                "}\n");
----------------
krasimir wrote:
> This looks a bit suspicious: I'd expect a break before the first arg to be 
> forced only when there exists a multiline (after formatting) lambda 
> expression arg. Is this (multiline vs. lambdas fitting 1 line) something that 
> we (can) differentiate with respect to? djasper@ might have an insight on 
> this.
Well, yes, I can see where you are coming from - the lambda is short and would 
fit. Unfortunately, I am not sure how to implement this nuance... I think I'd 
need to get the length of the unwrapped line and then check whether it fits in 
TokenUnnotator.cc....

Also, I personally favor less indentation (i.e. full width for the lambda) as 
that prevents drastic reformat when the lambda body changes. (that's why this 
patch exists)


================
Comment at: unittests/Format/FormatTest.cpp:11657
                "}});");
-  verifyFormat("virtual aaaaaaaaaaaaaaaa(std::function<bool()> bbbbbbbbbbbb 
=\n"
-               "                             [&]() { return true; },\n"
-               "                         aaaaa aaaaaaaaa);");
+  verifyFormat("virtual aaaaaaaaaaaaaaaa(\n"
+               "    std::function<bool()> bbbbbbbbbbbb = [&]() { return true; 
},\n"
----------------
krasimir wrote:
> Similar to my previous comment, this forcing the std::function on a newline 
> here might not be what we want to end up with?
I think this change is in line with the updated/extended semantics - the extra 
arg forces the lambda to its own line and the introducer is kept with the 
preceding tokens.


================
Comment at: unittests/Format/FormatTest.cpp:11736
+  // line and there are no further args.
+  verifyFormat("function(1, [this, that] {\n"
+               "  //\n"
----------------
krasimir wrote:
> Could we please have a test case where there are several args packed on the 
> first line, then a line break, then an arg, then a multiline lambda as a last 
> arg (illustrating that we don't pull the first arg down if there's only a 
> multiline lambda as the last arg):
> ```
> function(a, b, ccccccc,
>          d, [] () {
>   body
> });
> ```
Sure, that seems to work, but not in the way you expected :) I'll update the 
patch...

```
  verifyFormat("function(a, b, c, //\n"
               "         d, [this, that] {\n"
               "           //\n"
               "         });\n");
```


Repository:
  rC Clang

https://reviews.llvm.org/D52676



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

Reply via email to