[PATCH] D119599: Add option to align compound assignments like `+=`

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

Thanks for working on this!
Looks pretty good already.



================
Comment at: clang/include/clang/Format/Format.h:151
 
+  /// When aligning assignments, whether compound assignments like
+  /// ``+=``'s are aligned along with ``=``'s.
----------------
You need to update the RST files when updating the doc comments here. Please use https://github.com/llvm/llvm-project/blob/main/clang/docs/tools/dump_format_style.py for that.


================
Comment at: clang/include/clang/Format/Format.h:157
+  ///   a   &= 2;
+  ///   bbb  = 2;
+  ///
----------------
I guess it would be complicated to avoid adding an additional space here. I mean, it could be:
```
a  &= 2;
bbb = 2;
```
And with 3-char operators, there's one more space.


================
Comment at: clang/unittests/Format/FormatTest.cpp:16399
+  Alignment.AlignCompoundAssignments = true;
+  verifyFormat("aa <= 5;\n"
+               "a               &= 5;\n"
----------------
Please test spacing in separate `verifyFormat` cases as in the Format.h option description.


================
Comment at: clang/unittests/Format/FormatTest.cpp:16408
+               "dvsdsv          |= 5;\n"
+               "int dsvvdvsdvvv  = 123;",
+               Alignment);
----------------
Please test shift operators `<<=` and `>>=`.


================
Comment at: clang/unittests/Format/FormatTest.cpp:16872
+  // that things still get aligned.
+  Alignment.ColumnLimit = 20;
   EXPECT_EQ("int    a(int   x,\n"
----------------
Is it something that can be done/fixed separately?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119599/new/

https://reviews.llvm.org/D119599