[PATCH] D148131: Avoid unnecessarily aggressive line-breaking when using "LambdaBodyIndentation: OuterScope" with argument bin-packing.

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added a comment.

In D148131#4447426 <https://reviews.llvm.org/D148131#4447426>, @owenpan wrote:

> In D148131#4304960 <https://reviews.llvm.org/D148131#4304960>, @jp4a50 wrote:
>
>> Hey @MyDeveloperDay @owenpan . I'd appreciate if you guys have time to consider this change. It's the last significant issue my team and I are tracking that's blocking our adoption of clang-format for multiple codebases that use KJ <https://github.com/capnproto/capnproto/blob/master/kjdoc/style-guide.md> :)
>
> We probably should fix https://github.com/llvm/llvm-project/issues/44486 first.

Hi @owenpan . Thanks for your reply. Sorry that I missed it for a while - I stopped monitoring the review after a while and missed the email notification because I hadn't figured out how to make my LLVM email notifications only send me notifications related to my reviews.

I've had a bit of a look into https://github.com/llvm/llvm-project/issues/44486 and I can see why it's happening. It is caused by the unconditional, aggressive line-breaking introduced by https://reviews.llvm.org/D52676 as mkurdej notes on the github issue.

Unfortunately I think the fact that the changes in https://reviews.llvm.org/D52676 were implemented in the TokenAnnotator (rather than ContinuationIndenter) means this will be a non-trivial fix - I don't think the decision to force these line breaks should have been implemented here as there is not enough context to handle edge cases like the one raised in https://github.com/llvm/llvm-project/issues/44486.

I'll have a go at moving the implementation of this behaviour into ContinuationIndenter to see if it allows me to fix the issue. My rough idea is to get the OptimizingLineFormatter to consider either breaking or not breaking before the first argument, but when we don't break before the first argument, disallow line-breaking before any other arguments (in all cases that we have lambda arguments except when there is a single lambda that is the last argument).

Hopefully that should allow us to maintain the same logic as we currently have but also consider the additional case of having all arguments (including the lambdas) on the same line. I doubt it will be as simple as that but thought I'd give you a heads up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148131