[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Ok, I think I agree with both of you to a certain extent, but I also think this change as is, is not yet right.

First, it does too much. The original very first example in this CL is actually not produced by clang-format (or if it is, I don't know with which flag combination). It is a case where the lambda is the last parameter. For me, it actually produces:

  void f() {
    something.something.something.Method(some_arg, [] {
      // the code here incurs
      // excessive wrapping
      // such as
      Method(some_med_arg, some_med_arg);
      some_var = some_expr + something;
    });
  }

And that's an intentional optimization for a very common lambda use case. It reduces indentation even further and makes some coding patterns much nicer. I think (but haven't reproduced) that you patch might change the behavior there.

Second, I agree that the original behavior is inconsistent in a way that we have a special cases for when the lambda is the very first parameter, but somehow seem be forgetting about that when it's not the first parameter. I'd be ok with "fixing" that (it's not a clear-cut bug, but I think the alternative behavior would be cleaner). However, I don't think your patch is doing enough there. I think this should be irrespective of bin-packing (it's related but not quite the same thing), and it should also apply to multiline strings if AlwaysBreakBeforeMultilineStrings is true. It doesn't all have to be done in the same patch, but we should have a plan of what we want the end result to look like and should be willing to get there.

Maybe to start with, we need the complete test matrix so that we are definitely on the same page as to what we are trying to do. I imagine, we would need tests for a function with three parameters, where two of the parameters are short and one is a multiline lambda or a multiline string (and have the lambda be the first, second and third parameter). Then we might need those for both bin-packing and no-bin-packing configurations.


Repository:
  rC Clang

https://reviews.llvm.org/D52676