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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
