[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In https://reviews.llvm.org/D33589#931723, @Typz wrote:

> In https://reviews.llvm.org/D33589#925903, @klimek wrote:
>
> > I think this patch doesn't handle a couple of cases that I'd like to see handled. A counter-proposal with different trade-offs is in https://reviews.llvm.org/D40068.
>
>
> Can you provide more info on these cases? Are they all added to the tests of https://reviews.llvm.org/D40068?
>
> It may be simpler (though not to my eyes, I am not knowledgeable enough to really understand how you go this fixed...), and works fine for "almost correct" comments: e.g. when there are indeed just a few extra characters overall. But it still procudes strange result when each line of the (long) comment is too long, but not enough to trigger a line-wrap by itself.
>
> Since that version has landed already, not sure how to improve on this. I could probably rewrite my patch on master, but it seems a bit redundant. As a simpler fix, I could imagine adding a "total" overflow counter, to allow detecting the situation; but when this is detected (e.g. on subsequent lines) we would need to "backtrack" and revisit the initial decision...


Yes, I added all tests I was thinking of to the patch. Note that we can always go back on submitted patches / do things differently. As my patch fulfilled all your tests, I (perhaps incorrectly?) assumed it solved your use cases - can you give me a test case of what you would want to happen that doesn't happen with my patch?

Are you, for example, saying that in the case

  Limit; 13
  // foo bar baz foo bar baz foo bar baz
  you'd not want
  // foo bar baz
  // foo bar baz
  // foo bar baz
  If the overall penalty of the protruding tokens is - what? More than 1 break penalty? If you care about more than 3x break penalty (which I would think is correct), the local algorithm will work, as local decisions will make sure the overall penalty cannot be exceeded.


https://reviews.llvm.org/D33589