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

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

I have a slightly hard time grasping what this patch now actually does? Doesn't it simply try to decide whether or not to make a split locally be comparing the PenaltyBreakComment against the penalty for the access characters? If so, couldn't we simply do that as an implementation detail of breakProtrudingToken() without needing to let anything outside of it now and without introducing State.Reflow?



================
Comment at: lib/Format/ContinuationIndenter.cpp:1339
 
+unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
+                                                    LineState &State,
----------------
Can you create a patch that doesn't move the code around so much? Seems unnecessary and hard to review.


================
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);
----------------
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


https://reviews.llvm.org/D33589