r293755 - [clang-format] Fix regression about not aligning trailing comments in case they were previously aligned, but at different indent.

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Wed Feb  1 04:10:04 2017
New Revision: 293755

URL: http://llvm.org/viewvc/llvm-project?rev=293755&view=rev
Log:
[clang-format] Fix regression about not aligning trailing comments in case they were previously aligned, but at different indent.

Summary:
Comment reflower was adding untouchable tokens in case two consecutive comment lines are aligned in the source code. This disallows the whitespace manager to re-indent them later.

source:
```
int i = f(abc, // line 1
          d, // line 2
	     // line 3
	  b);
```
Since line 2 and line 3 are aligned, the reflower was marking line 3 as untouchable; however the three comment lines need to be re-aligned.
output before:
```
int i = f(abc, // line 1
          d,   // line 2
	     // line 3
	  b);
```
output after:
```
int i = f(abc, // line 1
          d,   // line 2
	       // line 3
	  b);
```

Reviewers: djasper

Reviewed By: djasper

Subscribers: sammccall, cfe-commits, klimek

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

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=293755&r1=293754&r2=293755&view=diff
==============================================================================
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Wed Feb  1 04:10:04 2017
@@ -803,18 +803,16 @@ void BreakableLineCommentSection::replac
           ContentColumn[LineIndex] -
           (Content[LineIndex].data() - Lines[LineIndex].data()) +
           (OriginalPrefix[LineIndex].size() - Prefix[LineIndex].size());
-      if (tokenAt(LineIndex).OriginalColumn != LineColumn) {
-        Whitespaces.replaceWhitespace(*Tokens[LineIndex],
-                                      /*Newlines=*/1,
-                                      /*Spaces=*/LineColumn,
-                                      /*StartOfTokenColumn=*/LineColumn,
-                                      /*InPPDirective=*/false);
-      } else {
-        // The whitespace preceding the first line of this token does not need
-        // to be touched.
-        Whitespaces.addUntouchableToken(tokenAt(LineIndex),
-                                        /*InPPDirective=*/false);
-      }
+
+      // We always want to create a replacement instead of adding an untouchable
+      // token, even if LineColumn is the same as the original column of the
+      // token. This is because WhitespaceManager doesn't align trailing
+      // comments if they are untouchable.
+      Whitespaces.replaceWhitespace(*Tokens[LineIndex],
+                                    /*Newlines=*/1,
+                                    /*Spaces=*/LineColumn,
+                                    /*StartOfTokenColumn=*/LineColumn,
+                                    /*InPPDirective=*/false);
     }
   }
   if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=293755&r1=293754&r2=293755&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Feb  1 04:10:04 2017
@@ -2238,7 +2238,7 @@ TEST_F(FormatTest, ReflowsComments) {
   EXPECT_EQ("int i; // This long\n"
             "       // line gets\n"
             "       // broken.\n"
-            "       //  \n"
+            "       //\n"
             "       // keep.\n",
             format("int i; // This long line gets broken.\n"
                    "       //  \n"
@@ -11741,7 +11741,7 @@ TEST_F(ReplacementTest, SortIncludesAfte
   EXPECT_EQ(Expected, *Result);
 }
 
-TEST_F(FormatTest, AllignTrailingComments) {
+TEST_F(FormatTest, AlignTrailingComments) {
   EXPECT_EQ("#define MACRO(V)                       \\\n"
             "  V(Rt2) /* one more char */           \\\n"
             "  V(Rs)  /* than here  */              \\\n"
@@ -11751,6 +11751,15 @@ TEST_F(FormatTest, AllignTrailingComment
                    "V(Rs) /* than here  */    \\\n"
                    "/* comment 3 */         \\\n",
                    getLLVMStyleWithColumns(40)));
+  EXPECT_EQ("int i = f(abc, // line 1\n"
+            "          d,   // line 2\n"
+            "               // line 3\n"
+            "          b);",
+            format("int i = f(abc, // line 1\n"
+                   "          d, // line 2\n"
+                   "             // line 3\n"
+                   "          b);",
+                   getLLVMStyleWithColumns(40)));
 }
 } // end namespace
 } // end namespace format