[clang-format] Break before a sequence of line comments aligned with the next line.

Summary:
Make the comment alignment respect sections of line comments originally alinged
with the next token. Until now the decision how to break a continuous sequence
of line comments into sections was taken without reference to the next token.

source:
```
class A {
public: // comment about public
  // comment about a
  int a;
}
```

format before:
```
class A {
public: // comment about public
        // comment about a
  int a;
}
```

format after:
```
class A {
public: // comment about public
  // comment about a
  int a;
}
```

Reviewers: djasper, klimek

Reviewed By: klimek

Subscribers: cfe-commits, klimek

Differential Revision: https://reviews.llvm.org/D29626

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@294435 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Format/UnwrappedLineParser.cpp

@@ -2220,13 +2220,71 @@ const FormatToken *UnwrappedLineParser::getPreviousToken() {
   return Line->Tokens.back().Tok;
 }
 
+void UnwrappedLineParser::distributeComments(
+    const SmallVectorImpl<FormatToken *> &Comments,
+    const FormatToken *NextTok) {
+  // Whether or not a line comment token continues a line is controlled by
+  // the method continuesLineComment, with the following caveat:
+  //
+  // Define a trail of Comments to be a nonempty proper postfix of Comments such
+  // that each comment line from the trail is aligned with the next token, if
+  // the next token exists. If a trail exists, the beginning of the maximal
+  // trail is marked as a start of a new comment section.
+  //
+  // For example in this code:
+  //
+  // int a; // line about a
+  //   // line 1 about b
+  //   // line 2 about b
+  //   int b;
+  //
+  // the two lines about b form a maximal trail, so there are two sections, the
+  // first one consisting of the single comment "// line about a" and the
+  // second one consisting of the next two comments.
+  if (Comments.empty())
+    return;
+  bool ShouldPushCommentsInCurrentLine = true;
+  bool HasTrailAlignedWithNextToken = false;
+  unsigned StartOfTrailAlignedWithNextToken = 0;
+  if (NextTok) {
+    // We are skipping the first element intentionally.
+    for (unsigned i = Comments.size() - 1; i > 0; --i) {
+      if (Comments[i]->OriginalColumn == NextTok->OriginalColumn) {
+        HasTrailAlignedWithNextToken = true;
+        StartOfTrailAlignedWithNextToken = i;
+      }
+    }
+  }
+  for (unsigned i = 0, e = Comments.size(); i < e; ++i) {
+    FormatToken *FormatTok = Comments[i];
+    if (HasTrailAlignedWithNextToken &&
+        i == StartOfTrailAlignedWithNextToken) {
+      FormatTok->ContinuesLineCommentSection = false;
+    } else {
+      FormatTok->ContinuesLineCommentSection =
+          continuesLineComment(*FormatTok, *Line, CommentPragmasRegex);
+    }
+    if (!FormatTok->ContinuesLineCommentSection &&
+        (isOnNewLine(*FormatTok) || FormatTok->IsFirst)) {
+      ShouldPushCommentsInCurrentLine = false;
+    }
+    if (ShouldPushCommentsInCurrentLine) {
+      pushToken(FormatTok);
+    } else {
+      CommentsBeforeNextToken.push_back(FormatTok);
+    }
+  }
+}
+
 void UnwrappedLineParser::readToken() {
-  bool CommentsInCurrentLine = true;
+  SmallVector<FormatToken *, 1> Comments;
   do {
     FormatTok = Tokens->getNextToken();
     assert(FormatTok);
     while (!Line->InPPDirective && FormatTok->Tok.is(tok::hash) &&
            (FormatTok->HasUnescapedNewline || FormatTok->IsFirst)) {
+      distributeComments(Comments, FormatTok);
+      Comments.clear();
       // If there is an unfinished unwrapped line, we flush the preprocessor
       // directives only after that unwrapped line was finished later.
       bool SwitchToPreprocessorLines = !Line->Tokens.empty();
@@ -2256,20 +2314,17 @@ void UnwrappedLineParser::readToken() {
       continue;
     }
 
-    if (!FormatTok->Tok.is(tok::comment))
+    if (!FormatTok->Tok.is(tok::comment)) {
+      distributeComments(Comments, FormatTok);
+      Comments.clear();
       return;
-    FormatTok->ContinuesLineCommentSection =
-        continuesLineComment(*FormatTok, *Line, CommentPragmasRegex);
-    if (!FormatTok->ContinuesLineCommentSection &&
-        (isOnNewLine(*FormatTok) || FormatTok->IsFirst)) {
-      CommentsInCurrentLine = false;
-    }
-    if (CommentsInCurrentLine) {
-      pushToken(FormatTok);
-    } else {
-      CommentsBeforeNextToken.push_back(FormatTok);
     }
+
+    Comments.push_back(FormatTok);
   } while (!eof());
+
+  distributeComments(Comments, nullptr);
+  Comments.clear();
 }
 
 void UnwrappedLineParser::pushToken(FormatToken *Tok) {

lib/Format/UnwrappedLineParser.h

@@ -117,6 +117,21 @@ private:
   void nextToken();
   const FormatToken *getPreviousToken();
   void readToken();
+
+  // Decides which comment tokens should be added to the current line and which
+  // should be added as comments before the next token.
+  //
+  // Comments specifies the sequence of comment tokens to analyze. They get
+  // either pushed to the current line or added to the comments before the next
+  // token.
+  //
+  // NextTok specifies the next token. A null pointer NextTok is supported, and
+  // signifies either the absense of a next token, or that the next token
+  // shouldn't be taken into accunt for the analysis.
+  void distributeComments(const SmallVectorImpl<FormatToken *> &Comments,
+                          const FormatToken *NextTok);
+
+  // Adds the comment preceding the next token to unwrapped lines.
   void flushComments(bool NewlineBeforeNext);
   void pushToken(FormatToken *Tok);
   void calculateBraceTypes(bool ExpectClassBody = false);

unittests/Format/FormatTest.cpp

@@ -11929,6 +11929,122 @@ TEST_F(FormatTest, AlignTrailingComments) {
                    "\n"
                    "// long",
                    getLLVMStyleWithColumns(15)));
+
+  // Align comment line sections aligned with the next token with the next
+  // token.
+  EXPECT_EQ("class A {\n"
+            "public: // public comment\n"
+            "  // comment about a\n"
+            "  int a;\n"
+            "};",
+            format("class A {\n"
+                   "public: // public comment\n"
+                   "  // comment about a\n"
+                   "  int a;\n"
+                   "};",
+                   getLLVMStyleWithColumns(40)));
+  EXPECT_EQ("class A {\n"
+            "public: // public comment 1\n"
+            "        // public comment 2\n"
+            "  // comment 1 about a\n"
+            "  // comment 2 about a\n"
+            "  int a;\n"
+            "};",
+            format("class A {\n"
+                   "public: // public comment 1\n"
+                   "   // public comment 2\n"
+                   "  // comment 1 about a\n"
+                   "  // comment 2 about a\n"
+                   "  int a;\n"
+                   "};",
+                   getLLVMStyleWithColumns(40)));
+  EXPECT_EQ("int f(int n) { // comment line 1 on f\n"
+            "               // comment line 2 on f\n"
+            "  // comment line 1 before return\n"
+            "  // comment line 2 before return\n"
+            "  return n; // comment line 1 on return\n"
+            "            // comment line 2 on return\n"
+            "  // comment line 1 after return\n"
+            "}",
+            format("int f(int n) { // comment line 1 on f\n"
+                   "   // comment line 2 on f\n"
+                   "  // comment line 1 before return\n"
+                   "  // comment line 2 before return\n"
+                   "  return n; // comment line 1 on return\n"
+                   "   // comment line 2 on return\n"
+                   "  // comment line 1 after return\n"
+                   "}",
+                   getLLVMStyleWithColumns(40)));
+  EXPECT_EQ("int f(int n) {\n"
+            "  switch (n) { // comment line 1 on switch\n"
+            "               // comment line 2 on switch\n"
+            "  // comment line 1 before case 1\n"
+            "  // comment line 2 before case 1\n"
+            "  case 1: // comment line 1 on case 1\n"
+            "          // comment line 2 on case 1\n"
+            "    // comment line 1 before return 1\n"
+            "    // comment line 2 before return 1\n"
+            "    return 1; // comment line 1 on return 1\n"
+            "              // comment line 2 on return 1\n"
+            "  // comment line 1 before default\n"
+            "  // comment line 2 before default\n"
+            "  default: // comment line 1 on default\n"
+            "           // comment line 2 on default\n"
+            "    // comment line 1 before return 2\n"
+            "    return 2 * f(n - 1); // comment line 1 on return 2\n"
+            "                         // comment line 2 on return 2\n"
+            "    // comment line 1 after return\n"
+            "    // comment line 2 after return\n"
+            "  }\n"
+            "}",
+            format("int f(int n) {\n"
+                   "  switch (n) { // comment line 1 on switch\n"
+                   "              // comment line 2 on switch\n"
+                   "    // comment line 1 before case 1\n"
+                   "    // comment line 2 before case 1\n"
+                   "    case 1: // comment line 1 on case 1\n"
+                   "              // comment line 2 on case 1\n"
+                   "    // comment line 1 before return 1\n"
+                   "    // comment line 2 before return 1\n"
+                   "    return 1;  // comment line 1 on return 1\n"
+                   "             // comment line 2 on return 1\n"
+                   "    // comment line 1 before default\n"
+                   "    // comment line 2 before default\n"
+                   "    default:   // comment line 1 on default\n"
+                   "                // comment line 2 on default\n"
+                   "    // comment line 1 before return 2\n"
+                   "    return 2 * f(n - 1); // comment line 1 on return 2\n"
+                   "                        // comment line 2 on return 2\n"
+                   "    // comment line 1 after return\n"
+                   "     // comment line 2 after return\n"
+                   "  }\n"
+                   "}",
+                   getLLVMStyleWithColumns(80)));
+
+  // If all the lines in a sequence of line comments are aligned with the next
+  // token, the first line belongs to the previous token and the other lines
+  // belong to the next token.
+  EXPECT_EQ("int a; // line about a\n"
+            "long b;",
+            format("int a; // line about a\n"
+                   "       long b;",
+                   getLLVMStyleWithColumns(80)));
+  EXPECT_EQ("int a; // line about a\n"
+            "// line about b\n"
+            "long b;",
+            format("int a; // line about a\n"
+                   "       // line about b\n"
+                   "       long b;",
+                   getLLVMStyleWithColumns(80)));
+  EXPECT_EQ("int a; // line about a\n"
+            "// line 1 about b\n"
+            "// line 2 about b\n"
+            "long b;",
+            format("int a; // line about a\n"
+                   "       // line 1 about b\n"
+                   "       // line 2 about b\n"
+                   "       long b;",
+                   getLLVMStyleWithColumns(80)));
 }
 } // end namespace
 } // end namespace format