r293641 - [clang-format] Fix regression about adding leading whitespace to the content of line comments

Krasimir Georgiev via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 31 07:40:15 PST 2017


Author: krasimir
Date: Tue Jan 31 09:40:15 2017
New Revision: 293641

URL: http://llvm.org/viewvc/llvm-project?rev=293641&view=rev
Log:
[clang-format] Fix regression about adding leading whitespace to the content of line comments

Summary:
The reflower didn't measure precisely the line column of a line in the middle of
a line comment section that has a prefix that needs to be adapted.

source:
```
/// a
//b
```

format before:
```
/// a
 //b
```

format after:
```
/// a
// b
```

Reviewers: djasper

Reviewed By: djasper

Subscribers: sammccall, cfe-commits, klimek

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

Modified:
    cfe/trunk/lib/Format/BreakableToken.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=293641&r1=293640&r2=293641&view=diff
==============================================================================
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Tue Jan 31 09:40:15 2017
@@ -796,10 +796,13 @@ void BreakableLineCommentSection::replac
     } else {
       // This is the first line for the current token, but no reflow with the
       // previous token is necessary. However, we still may need to adjust the
-      // start column.
+      // start column. Note that ContentColumn[LineIndex] is the expected
+      // content column after a possible update to the prefix, hence the prefix
+      // length change is included.
       unsigned LineColumn =
           ContentColumn[LineIndex] -
-          (Content[LineIndex].data() - Lines[LineIndex].data());
+          (Content[LineIndex].data() - Lines[LineIndex].data()) +
+          (OriginalPrefix[LineIndex].size() - Prefix[LineIndex].size());
       if (tokenAt(LineIndex).OriginalColumn != LineColumn) {
         Whitespaces.replaceWhitespace(*Tokens[LineIndex],
                                       /*Newlines=*/1,
@@ -813,13 +816,14 @@ void BreakableLineCommentSection::replac
                                         /*InPPDirective=*/false);
       }
     }
-  } else if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {
-    // This is not the first line of the token. Adjust the prefix if necessary.
+  }
+  if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {
+    // Adjust the prefix if necessary.
 
     // Take care of the space possibly introduced after a decoration.
     assert(Prefix[LineIndex] == (OriginalPrefix[LineIndex] + " ").str() &&
-           "Expecting a block comment decoration to differ from original by "
-           "at most a space");
+           "Expecting a line comment prefix to differ from original by at most "
+           "a space");
     Whitespaces.replaceWhitespaceInToken(
         tokenAt(LineIndex), OriginalPrefix[LineIndex].size(), 0, "", "",
         /*InPPDirective=*/false, /*Newlines=*/0, /*Spaces=*/1);

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=293641&r1=293640&r2=293641&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Jan 31 09:40:15 2017
@@ -1371,6 +1371,25 @@ TEST_F(FormatTest, SplitsLongCxxComments
             format("//Even if it makes the line exceed the column limit",
                    getLLVMStyleWithColumns(51)));
   EXPECT_EQ("//--But not here", format("//--But not here", getLLVMStyle()));
+  EXPECT_EQ("/// line 1\n"
+            "// add leading whitespace",
+            format("/// line 1\n"
+                   "//add leading whitespace",
+                   getLLVMStyleWithColumns(30)));
+  EXPECT_EQ("/// line 1\n"
+            "/// line 2\n"
+            "//! line 3\n"
+            "//! line 4\n"
+            "//! line 5\n"
+            "// line 6\n"
+            "// line 7",
+            format("///line 1\n"
+                   "///line 2\n"
+                   "//! line 3\n"
+                   "//!line 4\n"
+                   "//!line 5\n"
+                   "// line 6\n"
+                   "//line 7", getLLVMStyleWithColumns(20)));
 
   EXPECT_EQ("// aa bb cc dd",
             format("// aa bb             cc dd                   ",




More information about the cfe-commits mailing list