r293891 - [clang-format] Fix breaking of comment sections in unwrapped lines containing newlines.

Krasimir Georgiev via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 2 06:36:51 PST 2017


Author: krasimir
Date: Thu Feb  2 08:36:50 2017
New Revision: 293891

URL: http://llvm.org/viewvc/llvm-project?rev=293891&view=rev
Log:
[clang-format] Fix breaking of comment sections in unwrapped lines containing newlines.

Summary:
The breaking of line comment sections was misaligning the case where the first comment line is on an unwrapped line containing newlines. In this case, the breaking column must be based on the source column of the last token that is preceded by a newline, not on the first token of the unwrapped line.

source:
```
enum A {
  a, // line 1
  // line 2
};
```
format before:
```
enum A {
  a, // line 1
     // line 2
};
```
format after:
```
enum A {
  a, // line 1
  // line 2
};
```

Reviewers: djasper

Reviewed By: djasper

Subscribers: cfe-commits, klimek

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

Modified:
    cfe/trunk/lib/Format/BreakableToken.cpp
    cfe/trunk/lib/Format/FormatToken.h
    cfe/trunk/lib/Format/UnwrappedLineParser.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=293891&r1=293890&r2=293891&view=diff
==============================================================================
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Thu Feb  2 08:36:50 2017
@@ -695,7 +695,7 @@ BreakableLineCommentSection::BreakableLi
       Content[i] = Content[i].substr(0, EndOfLine);
     }
     LineTok = CurrentTok->Next;
-    if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+    if (CurrentTok->Next && !CurrentTok->Next->ContinuesLineCommentSection) {
       // A line comment section needs to broken by a line comment that is
       // preceded by at least two newlines. Note that we put this break here
       // instead of breaking at a previous stage during parsing, since that

Modified: cfe/trunk/lib/Format/FormatToken.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=293891&r1=293890&r2=293891&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Thu Feb  2 08:36:50 2017
@@ -261,6 +261,11 @@ struct FormatToken {
   /// Only set if \c Type == \c TT_StartOfName.
   bool PartOfMultiVariableDeclStmt = false;
 
+  /// \brief Does this line comment continue a line comment section?
+  ///
+  /// Only set to true if \c Type == \c TT_LineComment.
+  bool ContinuesLineCommentSection = false;
+
   /// \brief If this is a bracket, this points to the matching one.
   FormatToken *MatchingParen = nullptr;
 

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=293891&r1=293890&r2=293891&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Thu Feb  2 08:36:50 2017
@@ -2066,31 +2066,58 @@ static bool continuesLineComment(const F
   // original start column of the min column token of the line.
   //
   // For example, the second line comment continues the first in these cases:
+  //
   // // first line
   // // second line
+  //
   // and:
+  //
   // // first line
   //  // second line
+  //
   // and:
+  //
   // int i; // first line
   //  // second line
+  //
   // and:
+  //
   // do { // first line
   //      // second line
   //   int i;
   // } while (true);
   //
+  // and:
+  //
+  // enum {
+  //   a, // first line
+  //    // second line
+  //   b
+  // };
+  //
   // The second line comment doesn't continue the first in these cases:
+  //
   //   // first line
   //  // second line
+  //
   // and:
+  //
   // int i; // first line
   // // second line
+  //
   // and:
+  //
   // do { // first line
   //   // second line
   //   int i;
   // } while (true);
+  //
+  // and:
+  //
+  // enum {
+  //   a, // first line
+  //   // second line
+  // };
   const FormatToken *MinColumnToken = Line.Tokens.front().Tok;
 
   // Scan for '{//'. If found, use the column of '{' as a min column for line
@@ -2103,6 +2130,11 @@ static bool continuesLineComment(const F
       break;
     }
     PreviousToken = Node.Tok;
+
+    // Grab the last newline preceding a token in this unwrapped line.
+    if (Node.Tok->NewlinesBefore > 0) {
+      MinColumnToken = Node.Tok;
+    }
   }
   if (PreviousToken && PreviousToken->is(tok::l_brace)) {
     MinColumnToken = PreviousToken;
@@ -2130,7 +2162,8 @@ void UnwrappedLineParser::flushComments(
     //
     // FIXME: Consider putting separate line comment sections as children to the
     // unwrapped line instead.
-    if (isOnNewLine(**I) && JustComments && !continuesLineComment(**I, *Line))
+    (*I)->ContinuesLineCommentSection = continuesLineComment(**I, *Line);
+    if (isOnNewLine(**I) && JustComments && !(*I)->ContinuesLineCommentSection)
       addUnwrappedLine();
     pushToken(*I);
   }
@@ -2196,7 +2229,9 @@ void UnwrappedLineParser::readToken() {
 
     if (!FormatTok->Tok.is(tok::comment))
       return;
-    if (!continuesLineComment(*FormatTok, *Line) &&
+    FormatTok->ContinuesLineCommentSection =
+        continuesLineComment(*FormatTok, *Line);
+    if (!FormatTok->ContinuesLineCommentSection &&
         (isOnNewLine(*FormatTok) || FormatTok->IsFirst)) {
       CommentsInCurrentLine = false;
     }

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=293891&r1=293890&r2=293891&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Feb  2 08:36:50 2017
@@ -951,7 +951,46 @@ TEST_F(FormatTest, UnderstandsSingleLine
                    "  c\n"
                    "};",
                    getLLVMStyleWithColumns(20)));
-
+  EXPECT_EQ("enum A {\n"
+            "  a, // line 1\n"
+            "  // line 2\n"
+            "};",
+            format("enum A {\n"
+                   "  a, // line 1\n"
+                   "  // line 2\n"
+                   "};",
+                   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("enum A {\n"
+            "  a, // line 1\n"
+            "     // line 2\n"
+            "};",
+            format("enum A {\n"
+                   "  a, // line 1\n"
+                   "   // line 2\n"
+                   "};",
+                   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("enum A {\n"
+            "  a, // line 1\n"
+            "  // line 2\n"
+            "  b\n"
+            "};",
+            format("enum A {\n"
+                   "  a, // line 1\n"
+                   "  // line 2\n"
+                   "  b\n"
+                   "};",
+                   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("enum A {\n"
+            "  a, // line 1\n"
+            "     // line 2\n"
+            "  b\n"
+            "};",
+            format("enum A {\n"
+                   "  a, // line 1\n"
+                   "   // line 2\n"
+                   "  b\n"
+                   "};",
+                   getLLVMStyleWithColumns(20)));
   verifyFormat(
       "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
       "    bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb; // Trailing comment");




More information about the cfe-commits mailing list