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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits