This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Emit absolute splits before lines for comments
ClosedPublic

Authored by krasimir on Aug 21 2017, 6:30 AM.

Details

Summary

This patch makes the splits emitted for the beginning of comment lines during
reformatting absolute. Previously, they were relative to the start of the
non-whitespace content of the line, which messes up further TailOffset
calculations in breakProtrudingToken. This fixes an assertion failure reported
in bug 34236: https://bugs.llvm.org/show_bug.cgi?id=34236.

Diff Detail

Repository
rL LLVM

Event Timeline

krasimir created this revision.Aug 21 2017, 6:30 AM
djasper added inline comments.Aug 21 2017, 11:51 PM
lib/Format/BreakableToken.cpp
553 ↗(On Diff #111965)

I think a split cannot be "Trimmed". Maybe "Result" or "NewSplit"?

557 ↗(On Diff #111965)

Why do you create a new split instead of:

TrimmedSplit += Content[LineIndex].size() - TrimmedContent.size();

?

unittests/Format/FormatTestComments.cpp
2785 ↗(On Diff #111965)

s/crusher/crasher/

:-D

Also, I'd just remove " bug 34236", seems redundant.

krasimir updated this revision to Diff 112147.Aug 22 2017, 3:57 AM
krasimir marked 3 inline comments as done.
  • Address review comments
djasper accepted this revision.Aug 23 2017, 5:13 AM

Looks good.

This revision is now accepted and ready to land.Aug 23 2017, 5:13 AM
This revision was automatically updated to reflect the committed changes.