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

Manuel Klimek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 23 07:03:48 PST 2017


klimek added a comment.

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

> > @klimek wrote:
> >  In the above example, we add 3 line breaks, and we'd add 1 (or more) additional line breaks when reflowing below the column limit.
> >  I agree that that can lead to different overall outcomes, but I don't see how the approach of this patch really fixes it - it will only ever reflow below the column limit, so it'll also lead to states for long lines where reflowing and leaving chars over the line limit might be the overall best choice (unless I'm missing something)?
>
> Definitely, this patch may not find the 'optimal' solution. What I mean is that we reduce the `PenaltyExcessCharacter` value to allow "occasionally" breaking the limit: instead of a hard limit, we want to allow lines to sometimes break the limit, but definitely not *all* the time. Both patch work fine when the code is "correct", i.e. there is indeed only a few lines which break the limit.
>
> But the local decision approach behaves really wrong IMHO when the code is formatted beyond the column: it can very easily reformat in such a way that the comment is reflown to what looks like a longer column limit. I currently have a ratio of 10 between  PenaltyBreakComment and PenaltyExcessCharacter (which empirically seemed to give a decent compromise, and match how our code is formatted; I need to try more with your patch, to see if we can get better values...): with this setting, a "non wrapped" comment will actually be reflown to ColumnLimit+10...


To me the reverse intuitively makes sense:
When I see that
// first line
// second line
// third line
stays the same, but
// first line second line third line
gets reflown into something different, I get confused :)

The only way I see to consistently solve this is to optimize the reflow breaks like we do the main algorithm.

> When we do indeed reflow, I think we may be stricter than this, to get something that really looks like it obeys the column limit. If this is 'optimal' in the sense that we may have some overflow still, that is fine, but really not the primary requirement IMHO.




https://reviews.llvm.org/D33589





More information about the cfe-commits mailing list