r248254 - clang-format: Fix alignConsecutiveAssignments.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Sep 22 04:32:00 2015
New Revision: 248254

URL: http://llvm.org/viewvc/llvm-project?rev=248254&view=rev
Log:
clang-format: Fix alignConsecutiveAssignments.

It wasn't correctly handling this case:

  int oneTwoThree = 123;
  int oneTwo      = 12;
  method();

Review: http://reviews.llvm.org/D12369
Patch by Beren Minor. Thank you!

Modified:
    cfe/trunk/lib/Format/WhitespaceManager.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=248254&r1=248253&r2=248254&view=diff
==============================================================================
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Tue Sep 22 04:32:00 2015
@@ -156,41 +156,53 @@ void WhitespaceManager::alignConsecutive
   unsigned EndOfSequence = 0;
   bool FoundAssignmentOnLine = false;
   bool FoundLeftParenOnLine = false;
-  unsigned CurrentLine = 0;
 
+  // Aligns a sequence of assignment tokens, on the MinColumn column.
+  //
+  // Sequences start from the first assignment token to align, and end at the
+  // first token of the first line that doesn't need to be aligned.
+  //
+  // We need to adjust the StartOfTokenColumn of each Change that is on a line
+  // containing any assignment to be aligned and located after such assignment
   auto AlignSequence = [&] {
-    alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
+    if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
+      alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
     MinColumn = 0;
     StartOfSequence = 0;
     EndOfSequence = 0;
   };
 
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-    if (Changes[i].NewlinesBefore != 0) {
-      CurrentLine += Changes[i].NewlinesBefore;
-      if (StartOfSequence > 0 &&
-          (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine)) {
-        EndOfSequence = i;
+    if (Changes[i].NewlinesBefore > 0) {
+      EndOfSequence = i;
+      // If there is a blank line or if the last line didn't contain any
+      // assignment, the sequence ends here.
+      if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine) {
+        // NB: In the latter case, the sequence should end at the beggining of
+        // the previous line, but it doesn't really matter as there is no
+        // assignment on it
         AlignSequence();
       }
+
       FoundAssignmentOnLine = false;
       FoundLeftParenOnLine = false;
     }
 
-    if ((Changes[i].Kind == tok::equal &&
-         (FoundAssignmentOnLine || ((Changes[i].NewlinesBefore > 0 ||
-                                     Changes[i + 1].NewlinesBefore > 0)))) ||
-        (!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren)) {
-      if (StartOfSequence > 0)
-        AlignSequence();
+    // If there is more than one "=" per line, or if the "=" appears first on
+    // the line of if it appears last, end the sequence
+    if (Changes[i].Kind == tok::equal &&
+        (FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
+         Changes[i + 1].NewlinesBefore > 0)) {
+      AlignSequence();
+    } else if (!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren) {
+      AlignSequence();
     } else if (Changes[i].Kind == tok::l_paren) {
       FoundLeftParenOnLine = true;
-      if (!FoundAssignmentOnLine && StartOfSequence > 0)
+      if (!FoundAssignmentOnLine)
         AlignSequence();
     } else if (!FoundAssignmentOnLine && !FoundLeftParenOnLine &&
                Changes[i].Kind == tok::equal) {
       FoundAssignmentOnLine = true;
-      EndOfSequence = i;
       if (StartOfSequence == 0)
         StartOfSequence = i;
 
@@ -199,36 +211,34 @@ void WhitespaceManager::alignConsecutive
     }
   }
 
-  if (StartOfSequence > 0) {
-    EndOfSequence = Changes.size();
-    AlignSequence();
-  }
+  EndOfSequence = Changes.size();
+  AlignSequence();
 }
 
 void WhitespaceManager::alignConsecutiveAssignments(unsigned Start,
                                                     unsigned End,
                                                     unsigned Column) {
-  bool AlignedAssignment = false;
-  int PreviousShift = 0;
+  bool FoundAssignmentOnLine = false;
+  int Shift = 0;
   for (unsigned i = Start; i != End; ++i) {
-    int Shift = 0;
-    if (Changes[i].NewlinesBefore > 0)
-      AlignedAssignment = false;
-    if (!AlignedAssignment && Changes[i].Kind == tok::equal) {
+    if (Changes[i].NewlinesBefore > 0) {
+      FoundAssignmentOnLine = false;
+      Shift = 0;
+    }
+
+    // If this is the first assignment to be aligned, remember by how many
+    // spaces it has to be shifted, so the rest of the changes on the line are
+    // shifted by the same amount
+    if (!FoundAssignmentOnLine && Changes[i].Kind == tok::equal) {
+      FoundAssignmentOnLine = true;
       Shift = Column - Changes[i].StartOfTokenColumn;
-      AlignedAssignment = true;
-      PreviousShift = Shift;
+      Changes[i].Spaces += Shift;
     }
+
     assert(Shift >= 0);
-    Changes[i].Spaces += Shift;
+    Changes[i].StartOfTokenColumn += Shift;
     if (i + 1 != Changes.size())
       Changes[i + 1].PreviousEndOfTokenColumn += Shift;
-    Changes[i].StartOfTokenColumn += Shift;
-    if (AlignedAssignment) {
-      Changes[i].StartOfTokenColumn += PreviousShift;
-      if (i + 1 != Changes.size())
-        Changes[i + 1].PreviousEndOfTokenColumn += PreviousShift;
-    }
   }
 }
 

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=248254&r1=248253&r2=248254&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Sep 22 04:32:00 2015
@@ -8540,6 +8540,10 @@ TEST_F(FormatTest, AlignConsecutiveAssig
                "int oneTwoThree = 123;\n"
                "int oneTwo      = 12;",
                Alignment);
+  verifyFormat("int oneTwoThree = 123;\n"
+               "int oneTwo      = 12;\n"
+               "method();\n",
+               Alignment);
   verifyFormat("int oneTwoThree = 123; // comment\n"
                "int oneTwo      = 12;  // comment",
                Alignment);