[clang-format] Refine trailing comment detection

[krasimir]

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.

[krasimir]

ping

[djasper]

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]

lib/Format/FormatToken.h

397

Yes.

unittests/Format/FormatTestComments.cpp

461

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

[krasimir]

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

lib/Format/FormatToken.h

struct FormatToken {
 
   bool isBinaryOperator() const {
     // Comma is a binary operator, but does not behave as such wrt. formatting.
     return getPrecedence() > prec::Comma;
   }
 
   bool isTrailingComment() const {
     return is(tok::comment) &&
-           (is(TT_LineComment) || !Next || Next->NewlinesBefore > 0);
+           (is(TT_LineComment) || !Next ||
+            (Next->NewlinesBefore > 0 &&
+             (!Previous ||
+              Previous->isOneOf(tok::comma, tok::equal, tok::l_brace) ||
+              Next->is(tok::r_brace))));
   }
 
   /// \brief Returns \c true if this is a keyword that can be used
   /// like a function call (e.g. sizeof, typeid, ...).
   bool isFunctionLikeKeyword() const {
     switch (Tok.getKind()) {
     case tok::kw_throw:
     case tok::kw_typeid:

unittests/Format/FormatTestComments.cpp

EXPECT_EQ(
       "}",
       format("void      aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
              "                      aaaaaaaaaaaaaaaaaa  ,\n"
              "    aaaaaaaaaaaaaaaaaa) {   /*aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa*/\n"
              "}"));
   verifyFormat("f(/* aaaaaaaaaaaaaaaaaa = */\n"
                "  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);");
 
+  verifyFormat("const /** comment comment */ A = B;",
+               getLLVMStyleWithColumns(40));
+
+  verifyFormat("const /** comment comment comment */ A =\n"
+               "    B;",
+               getLLVMStyleWithColumns(40));
+
+  verifyFormat("const /** comment comment comment comment */\n"
+               "    A = B;",
+               getLLVMStyleWithColumns(40));
+
   FormatStyle NoBinPacking = getLLVMStyle();
   NoBinPacking.BinPackParameters = false;
   verifyFormat("aaaaaaaa(/* parameter 1 */ aaaaaa,\n"
                "         /* parameter 2 */ aaaaaa,\n"
                "         /* parameter 3 */ aaaaaa,\n"
                "         /* parameter 4 */ aaaaaa);",
                NoBinPacking);