This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Look at NoLineBreak and NoLineBreakInOperand before breakProtrudingToken
ClosedPublic

Authored by krasimir on Mar 3 2017, 7:19 AM.

Details

Summary

This patch makes ContinuationIndenter call breakProtrudingToken only if
NoLineBreak and NoLineBreakInOperand is false.

Previously, clang-format required two runs to converge on the following example with 24 columns:
Note that the second operand shouldn't be splitted according to NoLineBreakInOperand, but the
token breaker doesn't take that into account:

func(a, "long long long long", c);

After first run:

func(a, "long long "
        "long long",
         c);

After second run, where NoLineBreakInOperand is taken into account:

func(a,
     "long long "
     "long long",
     c);

With the patch, clang-format now obtains in one run:

func(a,
     "long long long"
     "long",
     c);

which is a better token split overall.

Event Timeline

krasimir created this revision.Mar 3 2017, 7:19 AM
krasimir updated this revision to Diff 90481.Mar 3 2017, 7:21 AM
  • Remove file
krasimir edited the summary of this revision. (Show Details)Mar 3 2017, 7:28 AM
krasimir added a reviewer: djasper.
krasimir added a subscriber: cfe-commits.
djasper added inline comments.Mar 8 2017, 3:32 AM
lib/Format/ContinuationIndenter.cpp
852

I think we assume here and in many other place that the stack is never empty. E.g. there is no similar check in l.847 above. So, I'd remove this here, too. In the long run, we probably want to have a class properly wrapping the Stack.

krasimir updated this revision to Diff 90999.Mar 8 2017, 5:03 AM
  • Remove empty stack check
krasimir marked an inline comment as done.Mar 8 2017, 5:03 AM
djasper accepted this revision.Mar 8 2017, 5:05 AM

Looks good.

This revision is now accepted and ready to land.Mar 8 2017, 5:05 AM
This revision was automatically updated to reflect the committed changes.