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

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1339
 
+unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
+                                                    LineState &State,
----------------
djasper wrote:
> Can you create a patch that doesn't move the code around so much? Seems unnecessary and hard to review.
Moving the code around is an unfortunate requirement for this patch: we must actually do the "reflowing" twice, to find out which solution (actually reflowing or not) gives the least penalty.

Therefore the function that reflowing must be moved out of `breakProtrudingToken()`...

All I can do is try moving the new function after `breakProtrudingToken()`, maybe Phabricator will better show the change.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1446
+  // Do not count the penalty twice, it will be added afterwards
+  if (State.Column > getColumnLimit(State)) {
+    unsigned ExcessCharacters = State.Column - getColumnLimit(State);
----------------
djasper wrote:
> I believe that this is incorrect. reflowProtrudingToken counts the length of the unbreakable tail and here you just remove the penalty of the token itself. E.g. in:
> 
>   string s = f("aaa");
> 
> the ");" is the unbreakable tail of the stringl
This behavior has not changed : before the commit, the last token was not included in the penalty [c.f. `if` at line 1338 in original code].

To make the comparison significative, the last token's penalty is included in the penalty returned by `reflowProtrudingToken()` (hence the removal of the test at line 1260); and it is removed before returning from this function, to keep the same behavior as before.


https://reviews.llvm.org/D33589