[clang-format] Adds a formatter for aligning arrays of structs

[feg208]

Summary

This adds a new formatter to arrange array of struct initializers into neat columns

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

16391

What's missing is to format your last code with a column limit of 20 or so.

Regards using EXPECT_EQ:

EXPECT_EQ("test demo[] = {\n"
      "    {56, 23,    \"hello world i am a very long line that really, in any "
      "\"\n"
      "                \"just world, ought be split over multiple lines\" },\n"
      "    {-1, 93463, \"world\"                                          },\n"
      "    {7,  5,     \"!!\"                                             },\n"
      "};\n",
format("test demo[] = {{56, 23, \"hello world i am a very long line that really, in any just world, ought be split over multiple lines\"},{-1, 93463, \"world\"},{7, 5, \"!!\"},};", Style));

This should be done with ColumnLimit 0, 20, 100.

With verifyFormat you verify that the formatting is stable, but especially with no column limit there is often no stable.

[curdeius]

clang/docs/ReleaseNotes.rst

251–252

clang/lib/Format/UnwrappedLineFormatter.cpp

920–923 ↗

(On Diff #343534)

Having this in a function seems like a bad idea, it just makes the code less readable. Please inline.

clang/unittests/Format/FormatTest.cpp

16370

Personally, I'm not really fond of testing all the combinations of styles. It adds little value to those tests.
I find it interesting to find problematic edge cases (which then should be added to the test suite "manually").
Also, when a test like this fails, it would be hard to see with which style it failed.

16443

Please add a test with preprocessor directives, e.g.:

verifyFormat("struct test demo[] = {\n"
             "    {56, 23,    \"hello\" },\n"
             "#if X\n"
             "    {-1, 93463, \"world\" },\n"
             "#endif\n"
             "    {7,  5,     \"!!\"    }\n"
             "};\n",
             Style);

16451

Nit: "ought to be" :).

[feg208]

I think I have all the tests and requested review comments rolled up

clang/unittests/Format/FormatTest.cpp

16370

I think it could be constexpr (thats the direction I originally started which lead to much moaning about the lack of fold expressions) however getLLVMStyle() is not constexpr and it felt inappropriate for this commit to change the interface to that function even if the change were relatively harmless.

[feg208]

Added more tests and addressed review comments

[feg208]

Oops missed a request

[feg208]

Missed a request in the release notes

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

16370

Also, when a test like this fails, it would be hard to see with which style it failed.

That is a valid point. You should only test some combinations.

16483

But this line is longer than 20! This test should fail, shouldn't it?

[HazardyKnusperkeks]

Something which just came to my mind. Since you wrote your own LineFormatter, you have to add test cases for all kinds of indention and wrapping. It seems that the string wrapping does not happen, how about comments? Are classes, namespaces, functions, etc. correctly indented with various configurations?

Given my expectations I think the alignment has to happen differently, but I'm happy to be proven wrong.

[feg208]

I still need to handle the 20 character case appropriately

clang/unittests/Format/FormatTest.cpp

16370

I can certainly strip it down to corner cases. I guess I am not clear on which ones I should focus on. When I implemented this none of the combinations created problems. I could pick a few at random? But that doesn't feel like an improvement.

16483

It should. When I use the command line invocation I get different output

#include <array>
struct test {
  int a, b;
  const char *c;
};

int main(
    int,
    const char **) {
  std::array<
      struct test,
      3>
      demo;
  demo = std::array<
      struct test,
      3>{
      test{
          56, 23,
          "hello "
          "world i "
          "am a "
          "very "
          "long "
          "line "
          "that "
          "really, "
          "in any "
          "just "
          "world, "
          "ought "
          "to be "
          "split "
          "over "
          "multiple"
          " lines"},
      test{-1,
           93463,
           "world"},
      test{7, 5,
           "!!"},
  };
}

I will investigate

[feg208]

In D101868#2745942, @HazardyKnusperkeks wrote:

Something which just came to my mind. Since you wrote your own LineFormatter, you have to add test cases for all kinds of indention and wrapping. It seems that the string wrapping does not happen, how about comments? Are classes, namespaces, functions, etc. correctly indented with various configurations?

Given my expectations I think the alignment has to happen differently, but I'm happy to be proven wrong.

hmmmmmm I think you are correct. Or at least the path I am going down with getting the case of a 20 character column width is demanding adding a bunch of things that replicate what is already there. Does a more fruitful avenue look something like annotating the tokens?

[MyDeveloperDay]

This looks like a good start..

All your tests are 3x3 have you considered mixing it up a bit. i.e. 2x3, what is the impact on 1x5 and 5x1 ?

Also how about nested structs, I'm interested to see what happens

{56, 23, { "ABC", 35 }}
{57, 24, { "DEF", 36 }}

[feg208]

In D101868#2774152, @MyDeveloperDay wrote:

This looks like a good start..

Thanks. I am reworking it so I handle line breaking in a sane fashion and dropping the LineFormatter override since that really can't handle reformatting without reimplementing most of it

All your tests are 3x3 have you considered mixing it up a bit. i.e. 2x3, what is the impact on 1x5 and 5x1 ?

I am also adding a bunch more tests. I'll roll this in.

Also how about nested structs, I'm interested to see what happens

{56, 23, { "ABC", 35 }}
{57, 24, { "DEF", 36 }}

Will do. I added the braced initializer int constructor test to exercise the same code path. But it can't hurt to get this in there

[MyDeveloperDay]

This is a valiant effort, and I definitely don't want to discourage..especially as you shown a good understand of clang-format. Its been a long asked for feature.. but we should really ensure this covers the bulk of the complaints if possible..

For me I use this.. a search for clang-format off in github projects

https://github.com/search?q=clang-format+off

Status AbsGrad(const AttrSlice& attrs, FunctionDef* g) {
  // clang-format off
  return GradForUnaryCwise(g, {
      {{"sign"}, "Sign", {"x"}, {}, {"dy"}},
      {{"dx"}, "Mul", {"dy", "sign"}},
  });
  // clang-format on
}

https://github.com/lulzsec2012/tensorflow/blob/d68d869e397515655e9f41570f4db463df770563/tensorflow/core/ops/math_grad.cc

it would be my ideal if we could irradiate the need for these, as they heavily dominate.

[MyDeveloperDay]

clang/unittests/Format/FormatTest.cpp

16407

may be worth testing a case with comments at the end of each line

verifyFormat("struct test demo[3] = {\n"
                 "    {56, 23,    \"hello\" }, // first line\n"
                 "    {-1, 93463, \"world\" }, /* second line */\n"
                 "    {7,  5,     \"!!\"    } // at the end\n"
                 "};\n",
                 Style);

[feg208]

This reworks substantially this commit. I recognize there are lacking/broken
tests but I just would like to ensure that the general direction doesn't
seem likely to end in tears

[feg208]

Forgot to alter the documentation to reflect right justified column alignment

[feg208]

Adds some tests and fixes a broken test

[feg208]

Fixes/adds a test that yielded a seg fault and quiets some clang-tidy checks

[HazardyKnusperkeks]

In D101868#2775550, @feg208 wrote:

This reworks substantially this commit. I recognize there are lacking/broken
tests but I just would like to ensure that the general direction doesn't
seem likely to end in tears

I'm not deep enough in clang-format and currently have not enough time to check that in depth. But why are you right aligning?

I would love to have a right aligning for numbers (or even better aligning around the decimal dot) in all my code, but strings I wouldn't want to right align.

[feg208]

In D101868#2776633, @HazardyKnusperkeks wrote:

In D101868#2775550, @feg208 wrote:

This reworks substantially this commit. I recognize there are lacking/broken
tests but I just would like to ensure that the general direction doesn't
seem likely to end in tears

I'm not deep enough in clang-format and currently have not enough time to check that in depth. But why are you right aligning?

So the basic approach here is to add the column aligning at the point that the lines are broken to make sure that we can align to the indented, now broken, columns. The right alignment was the easier path (at the moment) since the spaces attached to tokens in the change list proceeded the token and since I was calculating the column size with spaces based on a pointer attached to the token in the same column. To left align at each column I'd need to look at the adjacent column to determine the right number of spaces.

I would love to have a right aligning for numbers (or even better aligning around the decimal dot) in all my code, but strings I wouldn't want to right align.

I think we could certainly do that. I guess at this point (and this is somewhat motivated by the fact that I don't, and probably shouldn't, have commit rights the to llvm repo) I think if we want to move this commit forward we ought to agree on a set of changes and subsequent tests that we can all be comfortable with that would make this a viable bit of code. I don't think it has deep problems in the sense the prior one did and so should be amenable to laundry listing what we need and I'll move it forward. I think this set of tests should be added/fixed:

A test where the line is broken in the middle and/or first column (line breaking is really the sticky bit)
Fixing the test where the 100 column limit doesn't format correctly
Adding a test with several arrays to format and varying the other alignment options to make sure that doesn't confuse anything

I guess a final question I have would be, if we get this list sorted who can/would be capable of committing this change to the repo?

[feg208]

clang-tidy and clang-format changes

[HazardyKnusperkeks]

In D101868#2777423, @feg208 wrote:

In D101868#2776633, @HazardyKnusperkeks wrote:

In D101868#2775550, @feg208 wrote:

This reworks substantially this commit. I recognize there are lacking/broken
tests but I just would like to ensure that the general direction doesn't
seem likely to end in tears

I'm not deep enough in clang-format and currently have not enough time to check that in depth. But why are you right aligning?

So the basic approach here is to add the column aligning at the point that the lines are broken to make sure that we can align to the indented, now broken, columns. The right alignment was the easier path (at the moment) since the spaces attached to tokens in the change list proceeded the token and since I was calculating the column size with spaces based on a pointer attached to the token in the same column. To left align at each column I'd need to look at the adjacent column to determine the right number of spaces.

I would love to have a right aligning for numbers (or even better aligning around the decimal dot) in all my code, but strings I wouldn't want to right align.

I think we could certainly do that. I guess at this point (and this is somewhat motivated by the fact that I don't, and probably shouldn't, have commit rights the to llvm repo) I think if we want to move this commit forward we ought to agree on a set of changes and subsequent tests that we can all be comfortable with that would make this a viable bit of code. I don't think it has deep problems in the sense the prior one did and so should be amenable to laundry listing what we need and I'll move it forward. I think this set of tests should be added/fixed:

A test where the line is broken in the middle and/or first column (line breaking is really the sticky bit)
Fixing the test where the 100 column limit doesn't format correctly
Adding a test with several arrays to format and varying the other alignment options to make sure that doesn't confuse anything

I guess a final question I have would be, if we get this list sorted who can/would be capable of committing this change to the repo?

As said, I would really love more advanced alignment options, but I think we should keep to what clang-format does now, left aligning.

The tests seem to be reasonable, a combination of previous (mis)alignment and comments (end of line and in the middle) should be added.

clang/lib/Format/TokenAnnotator.cpp

2650

I don't know if there are already NOLINTs in the code and if there should be, on a different change I saw there are some messages from clang-tidy in the existing code which are ignored (not NOLINTed). That's something maybe someone with more experience can answer.

2662

Although I'm not a big fan of it, I think the LLVM style is no braces for one line if and else.

2681

This one I'm against, just do what clang-tidy says remove the else. (Others may disagree.)

[feg208]

The tests still need to be added

[feg208]

Addresses review comments

[feg208]

Still need to fix and add the tests I said I would but the comment tests are added

[HazardyKnusperkeks]

clang/lib/Format/FormatToken.cpp

86

Personal style, don't know about LLVM. Same below.

clang/unittests/Format/FormatTest.cpp

16472

And a comment after the value?

16478

Or at the line end?

[feg208]

I picked up most of these. One of the tests is already covered (I think) maybe I am misunderstanding

clang/unittests/Format/FormatTest.cpp

16478

I have this check up on lines 16384-16397

[feg208]

Rolls up review comments

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

16478

Sorry for that, I just looked at the diff between the revisions. I perform full review only if we are near the final decision and don't have all things (especially all test cases) in memory.

[feg208]

I am of the view that this is complete. Or at a minimum all the tests that have
been requested pass and I would be surprised if there were gross errors or other
major things I have missed

[feg208]

oops left some debugging messages in

[HazardyKnusperkeks]

I've added a few comments, and I would like to hear the opinion of others regarding the left or right alignment of the elements.

clang/lib/Format/ContinuationIndenter.cpp

603

Please don't remove the empty lines.

clang/lib/Format/TokenAnnotator.cpp

733

Capital letter for variables.

733

Where does the 2 come from? How do we know that + 2 is always applicable? There should be an explanation, and an assert.

On another note, I would prefer std::next().

2657–2659

There are still braces.

2663

I think if you revert the condition and drop the else (because of the break) makes it more readable.

clang/lib/Format/TokenAnnotator.h

38

Add a trailing comma, so that future additions do not need to change this line.

clang/lib/Format/WhitespaceManager.h

254 ↗

(On Diff #349324)

For iterators prefix increment is really better than postfix.

[feg208]

I rolled up the suggested changes.

clang/lib/Format/ContinuationIndenter.cpp

603

I am sorry. I was peeling back previous changes and must have overdone it

clang/lib/Format/TokenAnnotator.cpp

2657–2659

blech I thought I got those. Sorry.

[feg208]

Captured review comments

[feg208]

Missed a request

[feg208]

one remaining

[feg208]

Adds a simple lit test for some sanity checks

[HazardyKnusperkeks]

clang/lib/Format/TokenAnnotator.cpp

737–740

I actually meant so. Because now this is even safe if the iterators are not random access anymore in the future.

clang/lib/Format/WhitespaceManager.cpp

1136–1137 ↗

(On Diff #349434)

I don't know how to do this in LLVM style, but maybe so?

[feg208]

Grabs up some review comments and adds lit test

[feg208]

Got both of these

clang/lib/Format/TokenAnnotator.cpp

737–740

Oh I see. I sort of wondered about that but it didn't seem like a hill to die on.

clang/lib/Format/WhitespaceManager.cpp

1136–1137 ↗

(On Diff #349434)

So long as clang-format doesn't complain I am fine either way

[feg208]

Rebased against main

[feg208]

Had some invalid code in the lit test

[HazardyKnusperkeks]

Remains the issue with the alignment, I would like to know @MyDeveloperDay 's opinion on that. Should the values be right aligned, or left aligned? As far as I see all alignment in clang-format is left until now.

clang/lib/Format/WhitespaceManager.cpp

982 ↗

(On Diff #349730)

This is unneccessary. (But doesn't need a new revision until approved.)

1081 ↗

(On Diff #349730)

Same

[MyDeveloperDay]

In D101868#2799563, @HazardyKnusperkeks wrote:

Remains the issue with the alignment, I would like to know @MyDeveloperDay 's opinion on that. Should the values be right aligned, or left aligned? As far as I see all alignment in clang-format is left until now.

I think whatever you choose someone will ask for the opposite, maybe consider making it an option now while you are in the code and understand it.

[feg208]

Ok. Given @HazardyKnusperkeks comment I'll make the default be left alignment and update the tests accordingly

[MyDeveloperDay]

In D101868#2803670, @feg208 wrote:

Ok. Given @HazardyKnusperkeks comment I'll make the default be left alignment and update the tests accordingly

You could add AlignArrayOfStructuresMemberAlignment Left:Right you have the code to right align, your about to change it to left align, seems like you've done all the work.

[MyDeveloperDay]

Better still how about having

ArraryMemberAligmentEnum AlignArrayOfStructures

of "None, Left and Right"

I find boolean options don't stay boolean for long!

[curdeius]

In D101868#2805180, @MyDeveloperDay wrote:

Better still how about having

ArraryMemberAligmentEnum AlignArrayOfStructures

of "None, Left and Right"

I find boolean options don't stay boolean for long!

A strong +1.

[feg208]

In D101868#2805180, @MyDeveloperDay wrote:

Better still how about having

ArraryMemberAligmentEnum AlignArrayOfStructures

of "None, Left and Right"

I find boolean options don't stay boolean for long!

Yeah once I started doing the work this is exactly the path I am going down. Save the name. I should have it rolled up shortly.

[MyDeveloperDay]

Save the name. I should have it rolled up shortly.

You an keep the name, just change the type to be an enum

[feg208]

This alters the array alignment to allow for left alignment

[feg208]

Fixes from clang-tidy checks

[curdeius]

clang/docs/ClangFormatStyleOptions.rst

216–221

Don't forget to re-generate .rst.

[MyDeveloperDay]

clang/lib/Format/WhitespaceManager.h

233 ↗

(On Diff #350774)

Can ensure the comment starts with a capital and finishes with punctuation (I personally don't mind but I believe that is the convention)

274 ↗

(On Diff #350774)

291 ↗

(On Diff #350774)

310 ↗

(On Diff #350774)

313 ↗

(On Diff #350774)

[feg208]

Gets some of the formatting issues. Still chasing a test issue.

[feg208]

Got some changes. The failing test is an open issue

clang/docs/ClangFormatStyleOptions.rst

216–221

I am sorry. I am not entirely sure what to check here? Should I regenerate the docs for the site and just ensure that they look ok?

[feg208]

Fixes the busted test

[feg208]

Ick. Missed a semi colon in the test

[curdeius]

clang/docs/ClangFormatStyleOptions.rst

216–221

It seems that you modified Format.h but didn't re-run clang/docs/tools/dump_format_style.py (example for Left is actually right-aligned).

[feg208]

Regenerated the mangled ClangFormatStyle.rst file

[feg208]

addressed the remaining review comment

clang/docs/ClangFormatStyleOptions.rst

216–221

Got it. Sorry for the confusion.

[curdeius]

LGTM. That's a great piece work @feg208. Thank you!

I've added many nit comments, but I didn't do it for all code comments.
Please check that all comments are full phrases (with full stops :) ) before landing.
Some comments are in .rst but you know that you need to update Format.h and then regenerate .rst :).
Also, don't hesitate to mark comments as done.

Do you need somebody to land it on your behalf?

clang/docs/ClangFormatStyleOptions.rst

224–229

This repetition of demo struct seems not very useful.

258

Nit. This can be done when landing. No need to update the patch just for this.

clang/include/clang/Format/Format.h

93

clang/lib/Format/FormatToken.h

434–442

Nits.

clang/lib/Format/WhitespaceManager.cpp

994 ↗

(On Diff #350974)

1005 ↗

(On Diff #350974)

1013–1014 ↗

(On Diff #350974)

1062 ↗

(On Diff #350974)

1064 ↗

(On Diff #350974)

clang/lib/Format/WhitespaceManager.h

185–190 ↗

(On Diff #350974)

Nit.

265 ↗

(On Diff #350974)

Nit.

[feg208]

In D101868#2808826, @curdeius wrote:

LGTM. That's a great piece work @feg208. Thank you!

Awww thanks. I learned a lot from all the comments honestly. I appreciate the patience.

I've added many nit comments, but I didn't do it for all code comments.
Please check that all comments are full phrases (with full stops :) ) before landing.
Some comments are in .rst but you know that you need to update Format.h and then regenerate .rst :).
Also, don't hesitate to mark comments as done.

I'll roll these up.

Do you need somebody to land it on your behalf?

I do. I don't have commit rights

[feg208]

Rolls up the remaining review comments

[feg208]

All the review comments are addressed

[MyDeveloperDay]

LGTM

[feg208]

If I can get someone to submit this on my behalf I think we can call it a day.

[HazardyKnusperkeks]

In D101868#2810452, @feg208 wrote:

If I can get someone to submit this on my behalf I think we can call it a day.

Please post the name and email for the commit. And if no one else commits, I will do it on the weekend.

[feg208]

In D101868#2811290, @HazardyKnusperkeks wrote:

In D101868#2810452, @feg208 wrote:

If I can get someone to submit this on my behalf I think we can call it a day.

Please post the name and email for the commit. And if no one else commits, I will do it on the weekend.

Fred Grim [e-mail]

Thanks much

[vitalybuka]

clang/test/Format/struct-array-initializer.cpp

5 ↗

(On Diff #351734)

This test is very flaky https://lab.llvm.org/buildbot/#/builders/109

******************** TEST 'Clang :: Format/struct-array-initializer.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   grep -Ev "// *[A-Z-]+:" /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/struct-array-initializer.cpp    | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Right}" /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/struct-array-initializer.cpp    | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck -strict-whitespace -check-prefix=CHECK1 /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/struct-array-initializer.cpp
: 'RUN: at line 4';   grep -Ev "// *[A-Z-]+:" /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/struct-array-initializer.cpp    | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Left}" /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/struct-array-initializer.cpp    | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck -strict-whitespace -check-prefix=CHECK2 /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/struct-array-initializer.cpp
--
Exit Code: 141
Command Output (stderr):
--
+ : 'RUN: at line 1'
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang-format '-style={BasedOnStyle: LLVM, AlignArrayOfStructures: Right}' /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/struct-array-initializer.cpp
+ grep -Ev '// *[A-Z-]+:' /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/struct-array-initializer.cpp
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck -strict-whitespace -check-prefix=CHECK1 /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/struct-array-initializer.cpp
--
********************

[feg208]

clang/test/Format/struct-array-initializer.cpp

5 ↗

(On Diff #351734)

Yeah. I added you to a review to alter it to something I think will be less of an issue. Thanks by the by for getting the unused NetWidth variable

[lebedev.ri]

The test is still intermittently failing on bots.
Is there some non-determinism in the change?
It may be a good idea to revert this.

[MyDeveloperDay]

Do we actually need this lit test? Mostly we only add lit tests when we add a new command line argument, I feel your are just testing what we also test in the unittests.

[feg208]

I think, at this point, the sensible thing to do is to pull the test. As @MyDeveloperDay points out it isn't really adding any value above and beyond the unit tests. If it were non-determinism in the change itself the unit tests would catch it. They test the same formatting and test for non-determinism explicitly. Yet they've never failed. And there are quite a few of them.

[dyung]

Another +1 for it failing intermittently on the bot I watch over, llvm-clang-x86_64-sie-ubuntu-fast. Anything that can be done to stabilize the test or to remove it would be appreciated!

[feg208]

In D101868#2820075, @dyung wrote:

Another +1 for it failing intermittently on the bot I watch over, llvm-clang-x86_64-sie-ubuntu-fast. Anything that can be done to stabilize the test or to remove it would be appreciated!

I actually have two reviews up to remove the test https://reviews.llvm.org/D104295 or change it https://reviews.llvm.org/D104242 that have both been accepted but I don't have commit rights against llvm so I need to get someone to push them in. At this point removing the test is the least risky option. I am sorry for the delay and bother. Hopefully I can either get someone to put it in on my behalf or I'll get commit rights. I have followed the dev docs and emailed Chris.

[MyDeveloperDay]

There are a number of bugs logged against this feature, are you still around to look into them?

https://bugs.llvm.org/show_bug.cgi?id=51926
https://bugs.llvm.org/show_bug.cgi?id=51935

[feg208]

I am. I'll jump on this over the weekend. Sorry been a bit buried in my day job

clang/docs/ClangFormatStyleOptions.rst

* ``BAS_AlwaysBreak`` (in configuration: ``AlwaysBreak``)
     on a single line, e.g.:
 
     .. code-block:: c++
 
       someLongFunction(
           argument1, argument2);
 
 
+**AlignArrayOfStructures** (``bool``)
+  If ``true``, when using initialization for an array
+  of structs aligns the fields into right justified columns
+
+  .. code-block:: c
+  struct test demo[] =
+  {
+        {56,    23, "hello" },
+        {-1, 93463, "world" },
+        { 7,     5,    "!!" }
+  }
 
 **AlignConsecutiveAssignments** (``AlignConsecutiveStyle``)
   Style of aligning consecutive assignments.
 
   ``Consecutive`` will result in formattings like:
 
   .. code-block:: c++
 
     int a            = 1;
     int somelongname = 2;
     double c         = 3;
 
   Possible values:
 
   * ``ACS_None`` (in configuration: ``None``)
      Do not align assignments on consecutive lines.
 
   * ``ACS_Consecutive`` (in configuration: ``Consecutive``)
      Align assignments on consecutive lines. This will result in
      formattings like:
 
* ``ACS_AcrossEmptyLines`` (in configuration: ``AcrossEmptyLines``)
 
      .. code-block:: c++
 
        int a            = 1;
        int somelongname = 2;
        double c         = 3;
 
        int d            = 3;
        /* A comment. */
        double e = 4;
 
   * ``ACS_AcrossComments`` (in configuration: ``AcrossComments``)
      Same as ACS_Consecutive, but also spans over lines only containing
      comments, e.g.
 
      .. code-block:: c++
 

clang/docs/ReleaseNotes.rst

 
 - Option ``SpacesInAngles`` has been improved, it now accepts ``Leave`` value
   that allows to keep spaces where they are already present.
 
 - Option ``AllowShortIfStatementsOnASingleLine`` has been improved, it now
   accepts ``AllIfsAndElse`` value that allows to put "else if" and "else" short
   statements on a single line. (Fixes https://llvm.org/PR50019.)
 
 - ``git-clang-format`` no longer formats changes to symbolic links. (Fixes
   https://llvm.org/PR46992.)
 
+- Option ``AlignArrayOfStructure`` has been added to allow for ordering array-like
+  initializers.
+
 libclang
 --------
 
 - ...
 
 Static Analyzer
 ---------------
 

clang/include/clang/Format/Format.h

struct FormatStyle {
   };
 
   /// If ``true``, horizontally aligns arguments after an open bracket.
   ///
   /// This applies to round brackets (parentheses), angle brackets and square
   /// brackets.
   BracketAlignmentStyle AlignAfterOpenBracket;
 
+  /// if ``true``, when using initialization for an array of structs
+  /// aligns the fields into columns
+  /// \code
+  ///   struct test demo[] =
+  ///   {
+  ///       {56, 23,    "hello" },
+  ///       {-1, 93463, "world" },
+  ///       {7,  5,     "!!"    }
+  ///   }
+  /// \endcode
+  bool AlignArrayOfStructures;
+
   /// Styles for alignment of consecutive tokens. Tokens can be assignment signs
   /// (see
   /// ``AlignConsecutiveAssignments``), bitfield member separators (see
   /// ``AlignConsecutiveBitFields``), names in declarations (see
   /// ``AlignConsecutiveDeclarations``) or macro definitions (see
   /// ``AlignConsecutiveMacros``).
   enum AlignConsecutiveStyle {
     ACS_None,

   bool UseCRLF;
 
   /// The way to use tab characters in the resulting file.
   UseTabStyle UseTab;
 
   bool operator==(const FormatStyle &R) const {
     return AccessModifierOffset == R.AccessModifierOffset &&
            AlignAfterOpenBracket == R.AlignAfterOpenBracket &&
+           AlignArrayOfStructures == R.AlignArrayOfStructures &&
            AlignConsecutiveAssignments == R.AlignConsecutiveAssignments &&
            AlignConsecutiveBitFields == R.AlignConsecutiveBitFields &&
            AlignConsecutiveDeclarations == R.AlignConsecutiveDeclarations &&
            AlignConsecutiveMacros == R.AlignConsecutiveMacros &&
            AlignEscapedNewlines == R.AlignEscapedNewlines &&
            AlignOperands == R.AlignOperands &&
            AlignTrailingComments == R.AlignTrailingComments &&
            AllowAllArgumentsOnNextLine == R.AllowAllArgumentsOnNextLine &&

clang/lib/Format/ContinuationIndenter.h

private:
 
   /// Returns \c true if the next token starts a multiline string
   /// literal.
   ///
   /// This includes implicitly concatenated strings, strings that will be broken
   /// by clang-format and string literals with escaped newlines.
   bool nextIsMultilineString(const LineState &State);
 
+  /// make sure row positions is properly initialized and return the max width
+  unsigned setRowPositions(const FormatToken &Current, LineState &State);
+
   FormatStyle Style;
   const AdditionalKeywords &Keywords;
   const SourceManager &SourceMgr;
   WhitespaceManager &Whitespaces;
   encoding::Encoding Encoding;
   bool BinPackInconclusiveFunctions;
   llvm::Regex CommentPragmasRegex;
   const RawStringFormatStyleManager RawStringFormats;
struct LineState {
   /// The indent of the first token.
   unsigned FirstIndent;
 
   /// The line that is being formatted.
   ///
   /// Does not need to be considered for memoization because it doesn't change.
   const AnnotatedLine *Line;
 
+  /// The column position of the end of each of the row elements
+  /// for array initializers
+  std::vector<std::pair<unsigned, unsigned>> RowPositions;
+
+  /// Keep track of the current column index in array initializers
+  unsigned CurrentColumnIndex = 0U;
+
   /// Comparison operator to be able to used \c LineState in \c map.
   bool operator<(const LineState &Other) const {
     if (NextToken != Other.NextToken)
       return NextToken < Other.NextToken;
     if (Column != Other.Column)
       return Column < Other.Column;
     if (LineContainsContinuedForLoopSection !=
         Other.LineContainsContinuedForLoopSection)
       return LineContainsContinuedForLoopSection;
     if (NoContinuation != Other.NoContinuation)
       return NoContinuation;
     if (StartOfLineLevel != Other.StartOfLineLevel)
       return StartOfLineLevel < Other.StartOfLineLevel;
     if (LowestLevelOnLine != Other.LowestLevelOnLine)
       return LowestLevelOnLine < Other.LowestLevelOnLine;
     if (StartOfStringLiteral != Other.StartOfStringLiteral)
       return StartOfStringLiteral < Other.StartOfStringLiteral;
     if (IgnoreStackForComparison || Other.IgnoreStackForComparison)
       return false;
     return Stack < Other.Stack;
   }
+
+  unsigned getLBraceDepth() const;
 };
 
 } // end namespace format
 } // end namespace clang
 
 #endif

clang/lib/Format/ContinuationIndenter.cpp

if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash &&
     // For preprocessor indent with tabs, State.Column will be 1 because of the
     // hash. This causes second-level indents onward to have an extra space
     // after the tabs. We avoid this misalignment by subtracting 1 from the
     // column value passed to replaceWhitespace().
     if (Style.UseTab != FormatStyle::UT_Never)
       PPColumnCorrection = -1;
   }
 
-  if (!DryRun)
+  if (!DryRun) {
+    if (Style.AlignArrayOfStructures &&
+        State.Line->Type == LT_ArrayOfStructInitializer) {
+      auto ColumnPos = State.Column;
+      if (Current.PreviousColumn != nullptr) {
+        auto MaxWidth = setRowPositions(Current, State);
+        Spaces = MaxWidth - Current.getThisColumnWidth();
+        Spaces += ((Current.ColumnIndex != 0) ? 1 : 0U);
+        if ((Current.hasPreviousSplitMax() ||
+             MaxWidth < Current.getMaximumColumnWidth()) &&
+            Current.ColumnIndex > 0) {
+          // We had a line get broken so net the previous
+          // column
+          auto LeftPosition =
+              State.RowPositions[Current.ColumnIndex - 1].first +
+              State.RowPositions[Current.ColumnIndex - 1].second - 1;
+          if (Spaces > LeftPosition)
+            Spaces -= LeftPosition;
+          else if (Spaces < LeftPosition)
+            Spaces = 1;
+        }
+        ColumnPos = State.RowPositions[Current.ColumnIndex].first;
+      } else if (Current.IsRowClosing) {
+        Spaces += 1;
+      }
+      Whitespaces.replaceWhitespace(Current, /*Newlines=*/0, Spaces, ColumnPos);
+    } else {
       Whitespaces.replaceWhitespace(Current, /*Newlines=*/0, Spaces,
                                     State.Column + Spaces + PPColumnCorrection);
-
+    }
+  }
   // If "BreakBeforeInheritanceComma" mode, don't break within the inheritance
   // declaration unless there is multiple inheritance.
   if (Style.BreakInheritanceList == FormatStyle::BILS_BeforeComma &&
       Current.is(TT_InheritanceColon))
     State.Stack.back().NoLineBreak = true;
   if (Style.BreakInheritanceList == FormatStyle::BILS_AfterColon &&
       Previous.is(TT_InheritanceColon))
     State.Stack.back().NoLineBreak = true;
if (Current.is(tok::r_brace) && Current.MatchingParen &&
       // on its own line, and thus not covered by UnwrappedLineFormatter's logic
       // about removing empty lines on closing blocks. Special case them here.
       MaxEmptyLinesToKeep = 1;
     }
     unsigned Newlines =
         std::max(1u, std::min(Current.NewlinesBefore, MaxEmptyLinesToKeep));
     bool ContinuePPDirective =
         State.Line->InPPDirective && State.Line->Type != LT_ImportStatement;
-    Whitespaces.replaceWhitespace(Current, Newlines, State.Column, State.Column,
-                                  State.Stack.back().IsAligned,
+    bool AlignArrayOfStructures =
+        (Style.AlignArrayOfStructures &&
+         State.Line->Type == LT_ArrayOfStructInitializer);
+    if (AlignArrayOfStructures) {
+      unsigned Depth = State.getLBraceDepth();
+      if (Current.PreviousColumn != nullptr) {
+        auto MaxWidth = setRowPositions(Current, State);
+        auto ColumnPos = State.RowPositions[Current.ColumnIndex].first;
+        auto Spaces = MaxWidth - Current.getThisColumnWidth();
+        // FIXME really we shouldn't have this test here
+        if (Current.ColumnIndex == 0 &&
+            (ColumnPos + Current.getThisColumnWidth()) < Style.ColumnLimit) {
+          Newlines = 0;
+          State.RowPositions[Current.ColumnIndex] =
+              std::make_pair(State.Column + 1, MaxWidth);
+        } else if (Current.ColumnIndex > 0 && Newlines > 0) {
+          // We are linebreaking at a column so set the position to the first
+          State.RowPositions[Current.ColumnIndex] =
+              std::make_pair(State.RowPositions[0].first,
+                             State.RowPositions[Current.ColumnIndex].second);
+          ColumnPos = State.RowPositions[Current.ColumnIndex].first;
+          Spaces += ColumnPos - 1;
+        }
+        Whitespaces.replaceWhitespace(Current, Newlines, Spaces, ColumnPos);
+      } else if (!Current.isOneOf(tok::r_brace, tok::comma, tok::l_brace) &&
+                 State.RowPositions.size() > State.CurrentColumnIndex &&
+                 Depth == 2) {
+        auto ColumnPos = State.RowPositions[State.CurrentColumnIndex].first;
+        Whitespaces.replaceWhitespace(Current, Newlines, ColumnPos - 1,
+                                      ColumnPos);
+      } else {
+        Whitespaces.replaceWhitespace(Current, Newlines, State.Column,
+                                      State.Column);
+      }
+    } else {
+      Whitespaces.replaceWhitespace(Current, Newlines, State.Column,
+                                    State.Column, State.Stack.back().IsAligned,
                                     ContinuePPDirective);
     }
+  }
 
   if (!Current.isTrailingComment())
     State.Stack.back().LastSpace = State.Column;
   if (Current.is(tok::lessless))
     // If we are breaking before a "<<", we always want to indent relative to
     // RHS. This is necessary only for "<<", as we special-case it and don't
     // always indent relative to the RHS.
     State.Stack.back().LastSpace += 3; // 3 -> width of "<< ".
if ((Text.endswith(Postfix = "\"") &&
         (Text.startswith(Prefix = "_T(\"") && Text.endswith(Postfix = "\")"))) {
       // We need this to address the case where there is an unbreakable tail
       // only if certain other formatting decisions have been taken. The
       // UnbreakableTailLength of Current is an overapproximation is that case
       // and we need to be correct here.
       unsigned UnbreakableTailLength = (State.NextToken && canBreak(State))
                                            ? 0
                                            : Current.UnbreakableTailLength;
+      if (Style.AlignArrayOfStructures &&
+          State.Line->Type == LT_ArrayOfStructInitializer) {
+        if (!State.RowPositions.empty()) {
+          StartColumn = State.RowPositions[0].first - 1;
+        }
+      }
       return std::make_unique<BreakableStringLiteral>(
           Current, StartColumn, Prefix, Postfix, UnbreakableTailLength,
           State.Line->InPPDirective, Encoding, Style);
     }
   } else if (Current.is(TT_BlockComment)) {
     if (!Style.ReflowComments ||
         // If a comment token switches formatting, like
         // /* clang-format on */, we don't want to break it further,
while (ContentStartColumn + RemainingTokenColumns > ColumnLimit) {
       if (NewRemainingTokenColumns == RemainingTokenColumns) {
         // FIXME: Do we need to adjust the penalty?
         break;
       }
       assert(NewRemainingTokenColumns < RemainingTokenColumns);
 
       LLVM_DEBUG(llvm::dbgs() << "    Breaking at: " << TailOffset + Split.first
                               << ", " << Split.second << "\n");
-      if (!DryRun)
+      if (!DryRun) {
+        if (Style.AlignArrayOfStructures &&
+            State.Line->Type == LT_ArrayOfStructInitializer) {
+          // So at this point we have a initializer. We need to make sure the
+          // related column tokens are updated as well as any changes already
+          // inflight linked to those tokens
+          auto *CurrentWritableToken = State.NextToken->Previous;
+          for (; CurrentWritableToken != &Current;
+               CurrentWritableToken = CurrentWritableToken->Previous) {
+          }
+          assert(CurrentWritableToken != nullptr);
+          CurrentWritableToken->SplitMax =
+              CurrentWritableToken->TokenText.substr(TailOffset + Split.first)
+                  .size();
+        }
         Token->insertBreak(LineIndex, TailOffset, Split, ContentIndent,
                            Whitespaces);
-
+      }
       Penalty += NewBreakPenalty;
       TailOffset += Split.first + Split.second;
       RemainingTokenColumns = NewRemainingTokenColumns;
       BreakInserted = true;
       NewBreakBefore = true;
     }
     // In case there's another line, prepare the state for the start of the next
     // line.
if (Current.getNextNonComment() &&
     return true; // Implicit concatenation.
   if (Style.ColumnLimit != 0 && Style.BreakStringLiterals &&
       State.Column + Current.ColumnWidth + Current.UnbreakableTailLength >
           Style.ColumnLimit)
     return true; // String will be split.
   return false;
 }
 
+unsigned ContinuationIndenter::setRowPositions(const FormatToken &Current,
+                                               LineState &State) {
+  auto MaxWidth = Current.getMaximumColumnWidth(Style.ColumnLimit);
+  State.CurrentColumnIndex = Current.ColumnIndex;
+  if (State.RowPositions.size() <= Current.ColumnIndex)
+    State.RowPositions.push_back(std::make_pair(State.Column, MaxWidth));
+  else if (State.RowPositions[Current.ColumnIndex].second != MaxWidth) {
+    State.RowPositions[Current.ColumnIndex] =
+        std::make_pair(State.Column, MaxWidth);
+  }
+  return MaxWidth;
+}
+
+unsigned LineState::getLBraceDepth() const {
+  unsigned Depth = 0U;
+  for (const auto &PState : Stack) {
+    if (PState.Tok != nullptr)
+      Depth += (PState.Tok->is(tok::l_brace)) ? 1 : 0;
+  }
+  return Depth;
+}
+
 } // namespace format
 } // namespace clang

clang/lib/Format/Format.cpp

if (!IO.outputting()) {
                      Style.IndentWrappedFunctionNames);
       IO.mapOptional("PointerBindsToType", Style.PointerAlignment);
       IO.mapOptional("SpaceAfterControlStatementKeyword",
                      Style.SpaceBeforeParens);
     }
 
     IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);
     IO.mapOptional("AlignAfterOpenBracket", Style.AlignAfterOpenBracket);
+    IO.mapOptional("AlignArrayOfStructures", Style.AlignArrayOfStructures);
     IO.mapOptional("AlignConsecutiveMacros", Style.AlignConsecutiveMacros);
     IO.mapOptional("AlignConsecutiveAssignments",
                    Style.AlignConsecutiveAssignments);
     IO.mapOptional("AlignConsecutiveBitFields",
                    Style.AlignConsecutiveBitFields);
     IO.mapOptional("AlignConsecutiveDeclarations",
                    Style.AlignConsecutiveDeclarations);
     IO.mapOptional("AlignEscapedNewlines", Style.AlignEscapedNewlines);

 
 FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   FormatStyle LLVMStyle;
   LLVMStyle.InheritsParentConfig = false;
   LLVMStyle.Language = Language;
   LLVMStyle.AccessModifierOffset = -2;
   LLVMStyle.AlignEscapedNewlines = FormatStyle::ENAS_Right;
   LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
+  LLVMStyle.AlignArrayOfStructures = false;
   LLVMStyle.AlignOperands = FormatStyle::OAS_Align;
   LLVMStyle.AlignTrailingComments = true;
   LLVMStyle.AlignConsecutiveAssignments = FormatStyle::ACS_None;
   LLVMStyle.AlignConsecutiveBitFields = FormatStyle::ACS_None;
   LLVMStyle.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
   LLVMStyle.AlignConsecutiveMacros = FormatStyle::ACS_None;
   LLVMStyle.AllowAllArgumentsOnNextLine = true;
   LLVMStyle.AllowAllConstructorInitializersOnNextLine = true;

clang/lib/Format/FormatToken.h

public:
   FormatToken *MatchingParen = nullptr;
 
   /// The previous token in the unwrapped line.
   FormatToken *Previous = nullptr;
 
   /// The next token in the unwrapped line.
   FormatToken *Next = nullptr;
 
+  /// The previous vertical column element
+  FormatToken *PreviousColumn = nullptr;
+
+  /// Indicates that this is the closing bracket for a row
+  bool IsRowClosing = false;
+
+  /// We we split a token into multiples in the array initializer
+  /// case we need to track the size of the split for other columns
+  unsigned SplitMax = 0U;
+
+  /// For a token in a array initializer we need to track where in the
+  /// row it is so we can set offsets appropriately
+  unsigned ColumnIndex = 0U;
+
   /// If this token starts a block, this contains all the unwrapped lines
   /// in it.
   SmallVector<AnnotatedLine *, 1> Children;
 
   // Contains all attributes related to how this token takes part
   // in a configured macro expansion.
   llvm::Optional<MacroExpansion> MacroCtx;
 
const FormatToken *getNamespaceToken() const {
     return NamespaceTok &&
                    NamespaceTok->isOneOf(tok::kw_namespace, TT_NamespaceMacro)
                ? NamespaceTok
                : nullptr;
   }
 
   void copyFrom(const FormatToken &Tok) { *this = Tok; }
 
+  /// Returns the maximum width of all the columns above and below
+  /// this one
+  unsigned getMaximumColumnWidth(unsigned Limit = 0) const noexcept;
+
+  /// Returns the column width of multiple tokens in a array initializer
+  unsigned getThisColumnWidth(unsigned Limit = 0) const noexcept;
+
+  /// Returns a boolean indicated that a previous column element
+  /// was split
+  bool hasPreviousSplitMax() const noexcept;
+
 private:
   // Only allow copying via the explicit copyFrom method.
   FormatToken(const FormatToken &) = delete;
   FormatToken &operator=(const FormatToken &) = default;
 
   template <typename A, typename... Ts>
   bool startsSequenceInternal(A K1, Ts... Tokens) const {
     if (is(tok::comment) && Next)

clang/lib/Format/FormatToken.cpp

 /// roles.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "FormatToken.h"
 #include "ContinuationIndenter.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Debug.h"
+#include <algorithm>
 #include <climits>
 
 namespace clang {
 namespace format {
 
 const char *getTokenTypeName(TokenType Type) {
   static const char *const TokNames[] = {
 #define TYPE(X) #X,
bool FormatToken::isSimpleTypeSpecifier() const {
   case tok::kw_decltype:
   case tok::kw__Atomic:
     return true;
   default:
     return false;
   }
 }
 
+unsigned FormatToken::getMaximumColumnWidth(unsigned Limit) const noexcept {
+  unsigned MaxWidth = getThisColumnWidth();
+  auto *PColumn = PreviousColumn;
+  while (PColumn != nullptr && PColumn != this) {
+    MaxWidth = std::max(MaxWidth, PColumn->getThisColumnWidth(Limit));
+    PColumn = PColumn->PreviousColumn;
+  }
+  return MaxWidth;
+}
+
+unsigned FormatToken::getThisColumnWidth(unsigned Limit) const noexcept {
+  auto ColWidth =
+      (SplitMax > 0 && SplitMax < ColumnWidth) ? SplitMax : ColumnWidth;
+  ColWidth += is(tok::comment) ? 1 : 0;
+  const auto *NextColEntry = Next;
+  while (NextColEntry != nullptr &&
+         !NextColEntry->isOneOf(tok::r_brace, tok::comma)) {
+    if (NextColEntry->SplitMax > 0 &&
+        NextColEntry->SplitMax < NextColEntry->ColumnWidth) {
+      ColWidth += NextColEntry->SplitMax;
+    } else if (Limit > 0 && ColWidth + NextColEntry->ColumnWidth >= Limit) {
+      ColWidth = NextColEntry->ColumnWidth;
+    } else {
+      ColWidth += NextColEntry->ColumnWidth;
+      ColWidth += NextColEntry->is(tok::comment) ? 1 : 0;
+    }
+    NextColEntry = NextColEntry->Next;
+  }
+  return ColWidth;
+}
+
+bool FormatToken::hasPreviousSplitMax() const noexcept {
+  if (SplitMax > 0)
+    return true;
+  // We can't recurse here because there is no stopping
+  // condition in the case in which there is no SplitMaximum
+  const auto *PColumn = PreviousColumn;
+  while (PColumn != nullptr && PColumn != this) {
+    if (PColumn->SplitMax > 0)
+      return true;
+    PColumn = PColumn->PreviousColumn;
+  }
+  return false;
+}
+
 TokenRole::~TokenRole() {}
 
 void TokenRole::precomputeFormattingInfos(const FormatToken *Token) {}
 
 unsigned CommaSeparatedList::formatAfterToken(LineState &State,
                                               ContinuationIndenter *Indenter,
                                               bool DryRun) {
   if (State.NextToken == nullptr || !State.NextToken->Previous)

clang/lib/Format/TokenAnnotator.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LIB_FORMAT_TOKENANNOTATOR_H
 #define LLVM_CLANG_LIB_FORMAT_TOKENANNOTATOR_H
 
 #include "UnwrappedLineParser.h"
 #include "clang/Format/Format.h"
 
+#include <deque>
+#include <vector>
+
 namespace clang {
 class SourceManager;
 
 namespace format {
 
 enum LineType {
   LT_Invalid,
   LT_ImportStatement,
   LT_ObjCDecl, // An @interface, @implementation, or @protocol line.
   LT_ObjCMethodDecl,
   LT_ObjCProperty, // An @property line.
   LT_Other,
   LT_PreprocessorDirective,
-  LT_VirtualFunctionDecl
+  LT_VirtualFunctionDecl,
+  LT_ArrayOfStructInitializer
 };
 
 class AnnotatedLine {
 public:
   AnnotatedLine(const UnwrappedLine &Line)
       : First(Line.Tokens.front().Tok), Level(Line.Level),
         MatchingOpeningBlockLineIndex(Line.MatchingOpeningBlockLineIndex),
         MatchingClosingBlockLineIndex(Line.MatchingClosingBlockLineIndex),
private:
   bool canBreakBefore(const AnnotatedLine &Line, const FormatToken &Right);
 
   bool mustBreakForReturnType(const AnnotatedLine &Line) const;
 
   void printDebugInfo(const AnnotatedLine &Line);
 
   void calculateUnbreakableTailLengths(AnnotatedLine &Line);
 
+  void calculateArrayInitializerColumnList(AnnotatedLine &Line);
+
+  FormatToken *
+  calculateInitializerColumnList(AnnotatedLine &Line,
+                                 std::deque<FormatToken *> &&ColumnEntries,
+                                 FormatToken *CurrentToken, unsigned Depth);
+
   const FormatStyle &Style;
 
   const AdditionalKeywords &Keywords;
 };
 
 } // end namespace format
 } // end namespace clang
 
 #endif

clang/lib/Format/TokenAnnotator.cpp

while (CurrentToken) {
       FormatToken *Tok = CurrentToken;
       if (!consumeToken())
         return false;
       updateParameterCount(Left, Tok);
     }
     return false;
   }
 
+  bool couldBeInStructArrayInitializer() const {
+    const auto end = Contexts.rbegin() + 2;
+    auto last = Contexts.rbegin();
+    unsigned Depth = 0;
+    for (; last != end; ++last) {
+      if (last->ContextKind == tok::l_brace)
+        ++Depth;
+    }
+    return Depth == 2 && last->ContextKind != tok::l_brace;
+  }
+
   bool parseBrace() {
     if (CurrentToken) {
       FormatToken *Left = CurrentToken->Previous;
       Left->ParentBracket = Contexts.back().ContextKind;
-
       if (Contexts.back().CaretFound)
         Left->setType(TT_ObjCBlockLBrace);
       Contexts.back().CaretFound = false;
 
       ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);
       Contexts.back().ColonIsDictLiteral = true;
       if (Left->is(BK_BracedInit))
         Contexts.back().IsExpression = true;
       if (Style.Language == FormatStyle::LK_JavaScript && Left->Previous &&
           Left->Previous->is(TT_JsTypeColon))
         Contexts.back().IsExpression = false;
 
+      unsigned CommaCount = 0;
       while (CurrentToken) {
         if (CurrentToken->is(tok::r_brace)) {
           Left->MatchingParen = CurrentToken;
           CurrentToken->MatchingParen = Left;
+          if (Style.AlignArrayOfStructures) {
+            if (Left->ParentBracket == tok::l_brace &&
+                couldBeInStructArrayInitializer() && CommaCount > 0) {
+              Contexts.back().InStructArrayInitializer = true;
+            }
+          }
           next();
           return true;
         }
         if (CurrentToken->isOneOf(tok::r_paren, tok::r_square))
           return false;
         updateParameterCount(Left, CurrentToken);
         if (CurrentToken->isOneOf(tok::colon, tok::l_brace, tok::less)) {
           FormatToken *Previous = CurrentToken->getPreviousNonComment();
           if (Previous->is(TT_JsTypeOptionalQuestion))
             Previous = Previous->getPreviousNonComment();
           if ((CurrentToken->is(tok::colon) &&
                (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
               Style.Language == FormatStyle::LK_Proto ||
               Style.Language == FormatStyle::LK_TextProto) {
             Left->setType(TT_DictLiteral);
             if (Previous->Tok.getIdentifierInfo() ||
                 Previous->is(tok::string_literal))
               Previous->setType(TT_SelectorName);
           }
           if (CurrentToken->is(tok::colon) ||
               Style.Language == FormatStyle::LK_JavaScript)
             Left->setType(TT_DictLiteral);
         }
-        if (CurrentToken->is(tok::comma) &&
-            Style.Language == FormatStyle::LK_JavaScript)
+        if (CurrentToken->is(tok::comma)) {
+          if (Style.Language == FormatStyle::LK_JavaScript)
             Left->setType(TT_DictLiteral);
+          ++CommaCount;
+        }
         if (!consumeToken())
           return false;
       }
     }
     return true;
   }
 
   void updateParameterCount(FormatToken *Left, FormatToken *Current) {
LineType parseLine() {
 
     if (Line.startsWith(TT_ObjCMethodSpecifier)) {
       if (Contexts.back().FirstObjCSelectorName)
         Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
             Contexts.back().LongestObjCSelectorName;
       return LT_ObjCMethodDecl;
     }
 
+    for (const auto &ctx : Contexts) {
+      if (ctx.InStructArrayInitializer) {
+        return LT_ArrayOfStructInitializer;
+      }
+    }
+
     return LT_Other;
   }
 
 private:
   bool isClosureImportStatement(const FormatToken &Tok) {
     // FIXME: Closure-library specific stuff should not be hard-coded but be
     // configurable.
     return Tok.TokenText == "goog" && Tok.Next && Tok.Next->is(tok::period) &&
struct Context {
     bool CanBeExpression = true;
     bool InTemplateArgument = false;
     bool InCtorInitializer = false;
     bool InInheritanceList = false;
     bool CaretFound = false;
     bool IsForEachMacro = false;
     bool InCpp11AttributeSpecifier = false;
     bool InCSharpAttributeSpecifier = false;
+    bool InStructArrayInitializer = false;
   };
 
   /// Puts a new \c Context onto the stack \c Contexts for the lifetime
   /// of each instance.
   struct ScopedContextCreator {
     AnnotatingParser &P;
 
     ScopedContextCreator(AnnotatingParser &P, tok::TokenKind ContextKind,
                          unsigned Increase)
         : P(P) {
       P.Contexts.push_back(Context(ContextKind,
                                    P.Contexts.back().BindingStrength + Increase,
                                    P.Contexts.back().IsExpression));
     }
 
-    ~ScopedContextCreator() { P.Contexts.pop_back(); }
+    ~ScopedContextCreator() {
+      if (P.Style.AlignArrayOfStructures) {
+        if (P.Contexts.back().InStructArrayInitializer) {
+          P.Contexts.pop_back();
+          P.Contexts.back().InStructArrayInitializer = true;
+          return;
+        }
+      }
+      P.Contexts.pop_back();
+    }
   };
 
   void modifyContext(const FormatToken &Current) {
     if (Current.getPrecedence() == prec::Assignment &&
         !Line.First->isOneOf(tok::kw_template, tok::kw_using, tok::kw_return) &&
         // Type aliases use `type X = ...;` in TypeScript and can be exported
         // using `export type ...`.
         !(Style.Language == FormatStyle::LK_JavaScript &&
for (SmallVectorImpl<AnnotatedLine *>::iterator I = Line.Children.begin(),
     calculateFormattingInformation(**I);
   }
 
   Line.First->TotalLength =
       Line.First->IsMultiline ? Style.ColumnLimit
                               : Line.FirstStartColumn + Line.First->ColumnWidth;
   FormatToken *Current = Line.First->Next;
   bool InFunctionDecl = Line.MightBeFunctionDecl;
+  bool AlignArrayOfStructures = (Style.AlignArrayOfStructures &&
+                                 Line.Type == LT_ArrayOfStructInitializer);
+  if (AlignArrayOfStructures)
+    calculateArrayInitializerColumnList(Line);
+
   while (Current) {
     if (isFunctionDeclarationName(*Current, Line))
       Current->setType(TT_FunctionDeclarationName);
     if (Current->is(TT_LineComment)) {
       if (Current->Previous->is(BK_BracedInit) &&
           Current->Previous->opensScope())
         Current->SpacesRequiredBefore =
             (Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
while (Current) {
     } else {
       UnbreakableTailLength +=
           Current->ColumnWidth + Current->SpacesRequiredBefore;
     }
     Current = Current->Previous;
   }
 }
 
+void TokenAnnotator::calculateArrayInitializerColumnList(AnnotatedLine &Line) {
+  if (Line.First == Line.Last) {
+    return;
+  }
+  auto *CurrentToken = Line.First;
+  unsigned Depth = 0;
+  while (CurrentToken != nullptr && CurrentToken != Line.Last) {
+    if (CurrentToken->is(tok::l_brace))
+      CurrentToken = calculateInitializerColumnList(
+          Line, std::deque<FormatToken *>{}, CurrentToken->Next, Depth + 1);
+    else
+      CurrentToken = CurrentToken->Next;
+  }
+}
+
+// NOLINTNEXTLINE(misc-no-recursion)
+FormatToken *TokenAnnotator::calculateInitializerColumnList(
+    AnnotatedLine &Line, std::deque<FormatToken *> &&ColumnEntries,
+    FormatToken *CurrentToken, unsigned Depth) {
+  if (Depth == 1) {
+    CurrentToken->MustBreakBefore = true;
+    return calculateInitializerColumnList(Line, std::move(ColumnEntries),
+                                          CurrentToken, Depth + 1);
+  }
+
+  std::deque<FormatToken *> NextRow;
+  while (CurrentToken != nullptr && CurrentToken != Line.Last) {
+    if (CurrentToken->is(tok::l_brace))
+      ++Depth;
+    else if (CurrentToken->is(tok::r_brace))
+      --Depth;
+
+    if (CurrentToken->Next != nullptr) {
+      if (CurrentToken->is(tok::r_brace)) {
+        if (Depth == 2) {
+          if (CurrentToken->Next->is(tok::comma)) {
+            auto *NextLBrace = CurrentToken->Next->Next;
+            const auto *StopValue = CurrentToken->Next->getNextNonComment();
+            while (NextLBrace != nullptr && NextLBrace != StopValue)
+              NextLBrace = NextLBrace->Next;
+            if (NextLBrace != nullptr)
+              NextLBrace->MustBreakBefore = true;
+          }
+          CurrentToken->IsRowClosing = true;
+          return calculateInitializerColumnList(Line, std::move(NextRow),
+                                                CurrentToken->Next, Depth);
+        }
+
+        if (Depth == 1) {
+          // We are dropping out of the loop. So in this case take all of the
+          // entries in the last row and point the tails at the heads so we can
+          // loop around the columns ending where we began
+          while (!ColumnEntries.empty()) {
+            auto *LastToken = ColumnEntries.front();
+            if (LastToken->PreviousColumn != nullptr) {
+              auto *StartToken = LastToken->PreviousColumn;
+              while (StartToken->PreviousColumn != nullptr) {
+                StartToken = StartToken->PreviousColumn;
+              }
+              StartToken->PreviousColumn = LastToken;
+            }
+            ColumnEntries.pop_front();
+          }
+          CurrentToken->MustBreakBefore = true;
+        }
+      } else if (Depth == 3 &&
+                 CurrentToken->isOneOf(tok::comma, tok::l_brace)) {
+        CurrentToken->Next->ColumnIndex = NextRow.size();
+        NextRow.push_back(CurrentToken->Next);
+        if (!ColumnEntries.empty()) {
+          CurrentToken->Next->PreviousColumn = ColumnEntries.front();
+          ColumnEntries.pop_front();
+        }
+      }
+    }
+    CurrentToken = CurrentToken->Next;
+  }
+  return CurrentToken;
+}
+
 unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line,
                                       const FormatToken &Tok,
                                       bool InFunctionDecl) {
   const FormatToken &Left = *Tok.Previous;
   const FormatToken &Right = Tok;
 
   if (Left.is(tok::semi))
     return 0;

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, CatchExceptionReferenceBinding) {
   verifyFormat("void f() {\n"
                "  try {\n"
                "  } catch (const Exception &e) {\n"
                "  }\n"
                "}\n",
                getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructures) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = true;
+  Style.AlignConsecutiveAssignments =
+      FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+      FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+               "    {56,    23, \"hello\" },\n"
+               "    {-1, 93463, \"world\" },\n"
+               "    { 7,     5,    \"!!\" }\n"
+               "};\n",
+               Style);
+
+  verifyFormat("struct test demo[] = {\n"
+               "    {56,    23, \"hello\" }, // first line\n"
+               "    {-1, 93463, \"world\" }, // second line\n"
+               "    { 7,     5,    \"!!\" }  // third line\n"
+               "};\n",
+               Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+               "    { 56,    23, 21,       \"oh\" }, // first line\n"
+               "    { -1, 93463, 22,       \"my\" }, // second line\n"
+               "    {  7,     5,  1, \"goodness\" }  // third line\n"
+               "    {234,     5,  1, \"gracious\" }  // fourth line\n"
+               "};\n",
+               Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+               "    {56,    23, \"hello\" },\n"
+               "    {-1, 93463, \"world\" },\n"
+               "    { 7,     5,    \"!!\" }\n"
+               "};\n",
+               Style);
+  verifyFormat("struct test demo[3] = {\n"
+               "    {int{56},    23, \"hello\" },\n"
+               "    {int{-1}, 93463, \"world\" },\n"
+               "    { int{7},     5,    \"!!\" }\n"
+               "};\n",
+               Style);
+  verifyFormat("struct test demo[] = {\n"
+               "    {56,    23, \"hello\" },\n"
+               "    {-1, 93463, \"world\" },\n"
+               "    { 7,     5,    \"!!\" },\n"
+               "};\n",
+               Style);
+  verifyFormat("test demo[] = {\n"
+               "    {56,    23, \"hello\" },\n"
+               "    {-1, 93463, \"world\" },\n"
+               "    { 7,     5,    \"!!\" },\n"
+               "};\n",
+               Style);
+  verifyFormat("demo = std::array<struct test, 3>{\n"
+               "    test{56,    23, \"hello\" },\n"
+               "    test{-1, 93463, \"world\" },\n"
+               "    test{ 7,     5,    \"!!\" },\n"
+               "};\n",
+               Style);
+  verifyFormat("test demo[] = {\n"
+               "    {56,    23, \"hello\" },\n"
+               "#if X\n"
+               "    {-1, 93463, \"world\" },\n"
+               "#endif\n"
+               "    { 7,     5,    \"!!\" }\n"
+               "};\n",
+               Style);
+
+  verifyFormat("test demo[] = {\n"
+               "    { 7,    23,\n"
+               "    \"hello world i am a very long line that really, in any\"\n"
+               "    \"just world, ought to be split over multiple lines\" },\n"
+               "    {-1, 93463,                                 \"world\" },\n"
+               "    {56,     5,                                    \"!!\" }\n"
+               "};\n",
+               Style);
+
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+               "                                {{\"sign\"}, \"Sign\",  "
+               "{\"x\", \"dy\"} },\n"
+               "                                {  {\"dx\"},  \"Mul\", {\"dy\""
+               ", \"sign\"} },\n"
+               "                            });\n",
+               Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+      "test demo[] = {\n"
+      "    {56,    23, \"hello world i am a very long line that really, "
+      "in any just world, ought to be split over multiple lines\" },\n"
+      "    {-1, 93463,                                                  "
+      "                                                 \"world\" },\n"
+      "    { 7,     5,                                                  "
+      "                                                    \"!!\" },\n"
+      "};",
+      format("test demo[] = {{56, 23, \"hello world i am a very long line "
+             "that really, in any just world, ought to be split over multiple "
+             "lines\"},{-1, 93463, \"world\"},{7, 5, \"!!\"},};",
+             Style));
+
+  verifyFormat("test demo[] = {\n"
+               "    {56,    23, /* a comment */ \"hello\" },\n"
+               "    {-1, 93463,                 \"world\" },\n"
+               "    { 7,     5,                    \"!!\" }\n"
+               "};\n",
+               Style);
+
+  verifyFormat("test demo[] = {\n"
+               "    {56,    23,                    \"hello\" },\n"
+               "    {-1, 93463, \"world\" /* comment here */ },\n"
+               "    { 7,     5,                       \"!!\" }\n"
+               "};\n",
+               Style);
+
+  verifyFormat("test demo[] = {\n"
+               "    {56, /* a comment */ 23, \"hello\" },\n"
+               "    {-1,              93463, \"world\" },\n"
+               "    { 7,                  5,    \"!!\" }\n"
+               "};\n",
+               Style);
+
+  Style.ColumnLimit = 20;
+  EXPECT_EQ(
+      "demo = std::array<\n"
+      "    struct test, 3>{\n"
+      "    test{56,    23,\n"
+      "        \"hello \"\n"
+      "        \"world i \"\n"
+      "        \"am a very \"\n"
+      "        \"long line \"\n"
+      "        \"that \"\n"
+      "        \"really, \"\n"
+      "        \"in any \"\n"
+      "        \"just \"\n"
+      "        \"world, \"\n"
+      "        \"ought to \"\n"
+      "        \"be split \"\n"
+      "        \"over \"\n"
+      "        \"multiple \"\n"
+      "        \"lines\" },\n"
+      "    test{-1, 93463,\n"
+      "        \"world\" },\n"
+      "    test{ 7,     5,\n"
+      "           \"!!\" },\n"
+      "};",
+      format("demo = std::array<struct test, 3>{test{56, 23, \"hello world "
+             "i am a very long line that really, in any just world, ought "
+             "to be split over multiple lines\"},test{-1, 93463, \"world\"},"
+             "test{7, 5, \"!!\"},};",
+             Style));
+  // This caused a core dump by enabling Alignment in the LLVMStyle globally
+  Style = getLLVMStyleWithColumns(50);
+  Style.AlignArrayOfStructures = true;
+  verifyFormat("static A x = {\n"
+               "    {{init1, init2, init3, init4},\n"
+               "     {init1, init2, init3, init4} }\n"
+               "};",
+               Style);
+
+  // FIXME This is a case where the lines are obviously malformed
+  /*
+  Style.ColumnLimit = 100;
+  EXPECT_EQ(
+      "test demo[] = {\n"
+      "    {56,    23,\n"
+      "    \"hello world i am a very long line that really, in any just world"
+      ", ought to be split over \"\n"
+      "    \"multiple lines\"    },\n"
+      "    {-1, 93463, \"world\" },\n"
+      "    { 7,     5,    \"!!\" },\n"
+      "};",
+      format("test demo[] = {{56, 23, \"hello world i am a very long line "
+             "that really, in any just world, ought to be split over multiple "
+             "lines\"},{-1, 93463, \"world\"},{7, 5, \"!!\"},};",
+             Style));
+  */
+}
+
 TEST_F(FormatTest, UnderstandsPragmas) {
   verifyFormat("#pragma omp reduction(| : var)");
   verifyFormat("#pragma omp reduction(+ : var)");
 
   EXPECT_EQ("#pragma mark Any non-hyphenated or hyphenated string "
             "(including parentheses).",
             format("#pragma    mark   Any non-hyphenated or hyphenated string "
                    "(including parentheses)."));