[PATCH] D13513: [clang-format] Stop alignment sequences on open braces and parens when aligning assignments.

[Beren Minor via cfe-commits]

berenm created this revision.
berenm added a reviewer: djasper.
berenm added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

This was done correctly when aligning the declarations, but not when aligning 
assignments.

FIXME: The code between assignments and declarations alignment is roughly 
duplicated and
would benefit from factorization.

Bug 25090: https://llvm.org/bugs/show_bug.cgi?id=25090

http://reviews.llvm.org/D13513

Files:
  lib/Format/WhitespaceManager.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8649,6 +8649,19 @@
                "    someLooooooooooooooooongFunction();\n"
                "int j = 2;",
                Alignment);
+
+  verifyFormat("auto lambda = []() {\n"
+               "  auto i = 0;\n"
+               "  return 0;\n"
+               "};\n"
+               "int i  = 0;\n"
+               "auto v = type{\n"
+               "    i = 1,   //\n"
+               "    (i = 2), //\n"
+               "    i = 3    //\n"
+               "};",
+               Alignment);
+
   // FIXME: Should align all three assignments
   verifyFormat(
       "int i      = 1;\n"
@@ -8817,6 +8830,23 @@
                "    someLooooooooooooooooongFunction();\n"
                "int j = 2;",
                Alignment);
+
+  Alignment.AlignConsecutiveAssignments = true;
+  verifyFormat("auto lambda = []() {\n"
+               "  auto  ii = 0;\n"
+               "  float j  = 0;\n"
+               "  return 0;\n"
+               "};\n"
+               "int   i  = 0;\n"
+               "float i2 = 0;\n"
+               "auto  v  = type{\n"
+               "    i = 1,   //\n"
+               "    (i = 2), //\n"
+               "    i = 3    //\n"
+               "};",
+               Alignment);
+  Alignment.AlignConsecutiveAssignments = false;
+
   // FIXME: Should align all three declarations
   verifyFormat(
       "int      i = 1;\n"
Index: lib/Format/WhitespaceManager.cpp
===================================================================
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -153,6 +153,9 @@
 // a "=" is found on a line, extend the current sequence. If the current line
 // cannot be part of a sequence, e.g. because there is an empty line before it
 // or it contains non-assignments, finalize the previous sequence.
+//
+// FIXME: The code between assignment and declaration alignment is mostly
+// duplicated and would benefit from factorization.
 void WhitespaceManager::alignConsecutiveAssignments() {
   if (!Style.AlignConsecutiveAssignments)
     return;
@@ -162,6 +165,7 @@
   unsigned StartOfSequence = 0;
   unsigned EndOfSequence = 0;
   bool FoundAssignmentOnLine = false;
+  bool FoundLeftBraceOnLine = false;
   bool FoundLeftParenOnLine = false;
 
   // Aligns a sequence of assignment tokens, on the MinColumn column.
@@ -181,18 +185,21 @@
   };
 
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-    if (Changes[i].NewlinesBefore > 0) {
+    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) {
+      // If there is a blank line, if the last line didn't contain any
+      // assignment, or if we found an open brace or paren, the sequence ends
+      // here.
+      if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine ||
+          FoundLeftBraceOnLine || FoundLeftParenOnLine) {
         // 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;
+      FoundLeftBraceOnLine = false;
       FoundLeftParenOnLine = false;
     }
 
@@ -202,14 +209,24 @@
         (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::r_brace) {
+      if (!FoundLeftBraceOnLine)
+        AlignSequence();
+      FoundLeftBraceOnLine = false;
+    } else if (Changes[i].Kind == tok::l_brace) {
+      FoundLeftBraceOnLine = true;
+      if (!FoundAssignmentOnLine)
+        AlignSequence();
+    } else if (Changes[i].Kind == tok::r_paren) {
+      if (!FoundLeftParenOnLine)
+        AlignSequence();
+      FoundLeftParenOnLine = false;
     } else if (Changes[i].Kind == tok::l_paren) {
       FoundLeftParenOnLine = true;
       if (!FoundAssignmentOnLine)
         AlignSequence();
-    } else if (!FoundAssignmentOnLine && !FoundLeftParenOnLine &&
-               Changes[i].Kind == tok::equal) {
+    } else if (!FoundAssignmentOnLine && !FoundLeftBraceOnLine &&
+               !FoundLeftParenOnLine && Changes[i].Kind == tok::equal) {
       FoundAssignmentOnLine = true;
       if (StartOfSequence == 0)
         StartOfSequence = i;
@@ -267,6 +284,9 @@
 // current line cannot be part of a sequence, e.g. because there is an empty
 // line before it or it contains non-declarations, finalize the previous
 // sequence.
+//
+// FIXME: The code between assignment and declaration alignment is mostly
+// duplicated and would benefit from factorization.
 void WhitespaceManager::alignConsecutiveDeclarations() {
   if (!Style.AlignConsecutiveDeclarations)
     return;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits