Page MenuHomePhabricator

[clang-format] Refine trailing comment detection
AbandonedPublic

Authored by krasimir on Aug 11 2017, 7:09 AM.

Details

Reviewers
djasper
Summary

This patch fixes an non-idempotency issue connected with detection of trailing
comments. Consider formatting the following code with column limit at V:

                    V
const /* comment comment */ A = B;

The comment is not a trailing comment, breaking before it doesn't bring it under
the column limit. The formatter breaks after it, resulting in:

                    V
const /* comment comment */
    A = B;

For a next reformat, the formatter considers the comment as a trailing comment,
so it is free to break it further, resulting in:

                    V
const /* comment
         comment */
    A = B;

This patch improves this by refining the trailing comment detection, so that
the comment in the second version is not considered trailing anymore.

Event Timeline

krasimir created this revision.Aug 11 2017, 7:09 AM
djasper added inline comments.Aug 21 2017, 11:58 PM
lib/Format/FormatToken.h
397

Is this list coming from existing tests?

unittests/Format/FormatTestComments.cpp
461

I wonder whether instead we should just break the comment here, even if it is not a trailing one. Violating the column limit also seems bad. What happens if we do that (i.e. break non-trailing comments)?

krasimir marked an inline comment as done.Aug 22 2017, 5:12 AM
krasimir added inline comments.
lib/Format/FormatToken.h
397

Yes.

unittests/Format/FormatTestComments.cpp
461

Thanks! I've tried this approach here: https://reviews.llvm.org/D37007

krasimir abandoned this revision.Aug 22 2017, 7:42 AM
krasimir marked an inline comment as done.

This was superseded by https://reviews.llvm.org/D37007.