[clang-format]AlignConsecutiveAssignments

[owenpan]

Summary

Expanding AlignConsecutiveAssignments to compound assignments.

[PDoakORNL]

So it's been a week, is there an owner for clang/lib/Format?

[kristina]

Please upload your diffs with full context (use -U99999 when doing the diff) and update them. I don't think there is a code owner for this part but @klimek and @djasper do most reviews to do with clang-format, so you may consider adding them as reviewers.

[owenpan]

include/clang/Format/Format.h

91–93

These are invalid C++ examples.

lib/Format/WhitespaceManager.cpp

435–451

It would be simpler to just use isOneOf(tok::equal, tok::pipeequal, ...) here.

[krasimir]

include/clang/Format/Format.h

85

Please add tests for these. Also it's not clear from these examples how would a block of lines using assigments spanning different number of columns would be alined, as in:

aaa = 1;
bb += 2;
c <<= 3;

vs.

aaa = 1;
bb  += 2;
c   <<= 3;

I think this might deserve discussion by itself before this patch can get in (personally, I'm in favor of the first version where the right hand sides are alined).

[rolandschulz]

Is anyone still working on this or is a new author needed?

For what's it worth it, I'm in favor of the first version (right hand aligned) for different sized operators.

A matching bug exists at: https://bugs.llvm.org/show_bug.cgi?id=31681

[owenpan]

We habe AlignCompound now.

NOTE: Clang-Format Team Automated Review Comment

Your review contains a change to clang/include/clang/Format/Format.h but does not contain an update to ClangFormatStyleOptions.rst

ClangFormatStyleOptions.rst is generated via clang/docs/tools/dump_format_style.py, please run this to regenerate the .rst

You can validate that the rst is valid by running.

./docs/tools/dump_format_style.py
mkdir -p html
/usr/bin/sphinx-build -n ./docs ./html

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please try to ensure all code changes have a unit test (unless this is an NFC or refactoring, adding documentation etc..)

Add your unit tests in clang/unittests/Format and you can build with ninja FormatTests. We recommend using the verifyFormat(xxx) format of unit tests rather than EXPECT_EQ as this will ensure you change is tolerant to random whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can happen if you are trying to improve the annotation phase to ensure we are correctly identifying the type of a token, please add a token annotator test in TokenAnnotatorTest.cpp

include/clang/Format/Format.h

 
   /// If ``true``, aligns consecutive assignments.
   ///
-  /// This will align the assignment operators of consecutive lines. This
-  /// will result in formattings like
+  /// This will align the declaration names of consecutive lines and
+  /// matching assignment operators. This includes consecutive |=, +=
+  /// -=, /=, *=. This will result in formattings like
   /// \code
   ///   int aaaa = 12;
   ///   int b    = 23;
   ///   int ccc  = 23;
+  ///
+  ///   int ddd += 12;
+  ///   int ee  += 22;
+  ///   int f   += 23;
   /// \endcode
   bool AlignConsecutiveAssignments;
 

lib/Format/WhitespaceManager.cpp

 void WhitespaceManager::alignConsecutiveAssignments() {
   if (!Style.AlignConsecutiveAssignments)
     return;
+  std::vector<tok::TokenKind> assignment_tokens =
+    {tok::equal, tok::pipeequal, tok::caretequal, tok::percentequal,
+     tok::ampequal, tok::plusequal, tok::minusequal, tok::starequal,
+     tok::slashequal, tok::lesslessequal, tok::greatergreaterequal};
+  for (auto assignment_token : assignment_tokens)
+  {
+    AlignTokens(Style,
+    [&](const Change &C) {
+      // Do not align on equal signs that are first on a line.
+      if (C.NewlinesBefore > 0)
+        return false;
 
-  AlignTokens(Style,
-              [&](const Change &C) {
-                // Do not align on equal signs that are first on a line.
-                if (C.NewlinesBefore > 0)
-                  return false;
+      // Do not align on equal signs that are last on a line.
+      if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
+        return false;
 
-                // Do not align on equal signs that are last on a line.
-                if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
-                  return false;
-
-                return C.Tok->is(tok::equal);
-              },
-              Changes, /*StartAt=*/0);
+      return C.Tok->is(assignment_token);
+    },
+    Changes, /*StartAt=*/0);
+  }
 }
 
 void WhitespaceManager::alignConsecutiveDeclarations() {