[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1422
+                           Strict);
+    }
   }
----------------
krasimir wrote:
> The problem here is that we're calling breakProtrudingToken 3 times more than we used to. Seems like such a waste.
> @djasper: do you think this is fine?
If we're in DryRun mode (all through the optimization phase) and we don't actually leave around excess characters, we call this exactly once per protruding token, so the vast majority of cases will see no change.

We'll always have an additional call in non-dry-run mode, but I think that completely doesn't matter, and we'll have a second call if we actually do leave around excess characters - note that this only happens when the excess character penalty is low enough, so it won't actually affect the normal LLVM / Google styles.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1519
   if (!Token)
-    return 0;
+    return {0, true};
   assert(Token->getLineCount() > 0);
----------------
krasimir wrote:
> Why we return true here?
Argh, because I first had inverted the concept and changed it later - thanks for catching, will fix.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1773
                     Style.PenaltyExcessCharacter;
                 if (NewBreakPenalty < ExcessCharactersPenalty) {
                   Reflow = false;
----------------
krasimir wrote:
> Shouldn't we also be messing up something here in this patch?
This just switches reflowing to false, which will keep the original line break. This basically figures out whether we will later be able to break. That might lead to a non-perfect solution later, as we might reflow ourselves into a situation where we have to leave excess characters around, which we could have avoided if we kept under the limit, but I do not think that matteres in an observable way.


Repository:
  rC Clang

https://reviews.llvm.org/D40605