Page MenuHomePhabricator

[clang-format] Keep trailing preprocessor line comments separate from the following section comments
ClosedPublic

Authored by krasimir on May 22 2017, 1:44 AM.

Details

Summary

r303415 changed the way a sequence of line comments following a preprocessor
macro is handled, which has the unfortunate effect of aligning a trailing
preprocessor line comment and following unrelated section comments, so:

#ifdef A // comment about A
// section comment
#endif

gets turned into:

#ifdef A // comment about A
         // section comment
#endif

This patch fixes this by additionally checking the original start columns of
the line comments.

Diff Detail

Repository
rL LLVM

Event Timeline

krasimir created this revision.May 22 2017, 1:44 AM
djasper added inline comments.May 22 2017, 2:12 AM
lib/Format/UnwrappedLineParser.cpp
106 ↗(On Diff #99722)

Any chance of moving this logic to distributeComments? I think that's the place we previously tried to encapsulate this logic in. If not, this is fine for now.

unittests/Format/FormatTestComments.cpp
1030 ↗(On Diff #99722)

Would it make sense to also add a test case in which the #ifdef comment is actually continued?

krasimir updated this revision to Diff 99729.May 22 2017, 3:00 AM
krasimir marked an inline comment as done.
  • Address review comments
krasimir marked an inline comment as done.May 22 2017, 3:00 AM
krasimir added inline comments.
lib/Format/UnwrappedLineParser.cpp
106 ↗(On Diff #99722)

Because of the way parsing around macros works, moving this to distributeComments is gonna be super-tricky: I tried this approach first for a while and gave up. However, I can extract the common logic here and in continuesLineComment at least.

djasper accepted this revision.May 22 2017, 3:02 AM
djasper added inline comments.
lib/Format/UnwrappedLineParser.cpp
106 ↗(On Diff #99722)

Yeah. Much nicer. Thank you.

119 ↗(On Diff #99729)

Remove parentheses?

This revision is now accepted and ready to land.May 22 2017, 3:02 AM
krasimir updated this revision to Diff 99731.May 22 2017, 3:06 AM
krasimir marked an inline comment as done.
  • Remove parentheses
This revision was automatically updated to reflect the committed changes.