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

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

Reply via email to