from:"Fred Grim via Phabricator via cfe\-commits"

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

2021-05-21 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-05-22 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 347208.
feg208 added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16367,6 +16367,136 @@
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[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{\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);
+ /* FIXME Still broke
+  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));
+  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"
+

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

2021-05-22 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 347209.
feg208 added a comment.

Forgot to alter the documentation to reflect right justified column alignment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16367,6 +16367,136 @@
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[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{\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);
+ /* FIXME Still broke
+  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));
+  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"
+  "

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

2021-05-22 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 347211.
feg208 added a comment.

Adds some tests and fixes a broken test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16367,6 +16367,153 @@
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{\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[]

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

2021-05-23 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 347252.
feg208 added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16367,6 +16367,162 @@
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{\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"
+

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

2021-05-24 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

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?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-05-24 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 347419.
feg208 added a comment.

clang-tidy and clang-format changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16367,6 +16367,162 @@
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{\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[] = {{

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

2021-05-25 Thread Fred Grim via Phabricator via cfe-commits

feg208 marked 2 inline comments as done.
feg208 added a comment.

The tests still need to be added


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-05-25 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 347709.
feg208 added a comment.

Addresses review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16367,6 +16367,162 @@
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{\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, \"

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

2021-05-26 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 348020.
feg208 added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16367,6 +16367,176 @@
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{\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,  "
+  "\"!!\"

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

2021-05-27 Thread Fred Grim via Phabricator via cfe-commits

feg208 marked 3 inline comments as done.
feg208 added a comment.

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




Comment at: clang/unittests/Format/FormatTest.cpp:16478
+   "{56, /* a comment */ 23, \"hello\" },\n"
+   "{-1,  93463, \"world\" },\n"
+   "{ 7,  5,\"!!\" }\n"

HazardyKnusperkeks wrote:
> Or at the line end?
I have this check up on lines 16384-16397


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-05-27 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 348346.
feg208 marked an inline comment as done.
feg208 added a comment.

Rolls up review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16367,6 +16367,184 @@
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{\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"
+  "};",
+

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

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 349299.
feg208 added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
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{\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,  "
+  "

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

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 349324.
feg208 added a comment.

oops left some debugging messages in


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
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{\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 ju

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

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits

feg208 marked 6 inline comments as done.
feg208 added a comment.

I rolled up the suggested changes.




Comment at: clang/lib/Format/ContinuationIndenter.cpp:603
   State.Column + Spaces + PPColumnCorrection);
-
   // If "BreakBeforeInheritanceComma" mode, don't break within the inheritance

HazardyKnusperkeks wrote:
> Please don't remove the empty lines.
I am sorry. I was peeling back previous changes and must have overdone it



Comment at: clang/lib/Format/TokenAnnotator.cpp:2657-2659
+if (CurrentToken->is(tok::l_brace)) {
+  ++Depth;
+} else if (CurrentToken->is(tok::r_brace))

HazardyKnusperkeks wrote:
> There are still braces.
blech I thought I got those. Sorry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 349373.
feg208 marked 2 inline comments as done.
feg208 added a comment.

Captured review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
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{\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

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

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 349374.
feg208 marked an inline comment as done.
feg208 added a comment.

Missed a request


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
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{\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 s

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

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

one remaining


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 349434.
feg208 added a comment.

Adds a simple lit test for some sanity checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
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{\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 rea

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

2021-09-24 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-25 Thread Fred Grim via Phabricator via cfe-commits

feg208 created this revision.
feg208 added reviewers: HazardyKnusperkeks, MyDeveloperDay, curdeius, klimek, 
djasper, tinloaf.
feg208 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

bug 51926 identified an issue where a dangling comma caused the cell count to 
be to off by one


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110481

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17800,6 +17800,12 @@
 TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) {
   auto Style = getLLVMStyle();
   Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  verifyFormat("auto foo = Items{\n"
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"
+   "};\n",
+   Style);
   verifyFormat("struct test demo[] = {\n"
"{56, 23,\"hello\"},\n"
"{-1, 93463, \"world\"},\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1146,14 +1146,15 @@
   } else if (C.Tok->is(tok::comma)) {
 if (!Cells.empty())
   Cells.back().EndIndex = i;
-Cell++;
+if (C.Tok->Next->isNot(tok::r_brace))
+  Cell++;
   }
 } else if (Depth == 1) {
   if (C.Tok == MatchingParen) {
 if (!Cells.empty())
   Cells.back().EndIndex = i;
 Cells.push_back(CellDescription{i, ++Cell, i + 1, false, nullptr});
-CellCount = Cell + 1;
+CellCount = C.Tok->Previous->isNot(tok::comma) ? Cell + 1 : Cell;
 // Go to the next non-comment and ensure there is a break in front
 const auto *NextNonComment = C.Tok->getNextNonComment();
 while (NextNonComment->is(tok::comma))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17800,6 +17800,12 @@
 TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) {
   auto Style = getLLVMStyle();
   Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  verifyFormat("auto foo = Items{\n"
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"
+   "};\n",
+   Style);
   verifyFormat("struct test demo[] = {\n"
"{56, 23,\"hello\"},\n"
"{-1, 93463, \"world\"},\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1146,14 +1146,15 @@
   } else if (C.Tok->is(tok::comma)) {
 if (!Cells.empty())
   Cells.back().EndIndex = i;
-Cell++;
+if (C.Tok->Next->isNot(tok::r_brace))
+  Cell++;
   }
 } else if (Depth == 1) {
   if (C.Tok == MatchingParen) {
 if (!Cells.empty())
   Cells.back().EndIndex = i;
 Cells.push_back(CellDescription{i, ++Cell, i + 1, false, nullptr});
-CellCount = Cell + 1;
+CellCount = C.Tok->Previous->isNot(tok::comma) ? Cell + 1 : Cell;
 // Go to the next non-comment and ensure there is a break in front
 const auto *NextNonComment = C.Tok->getNextNonComment();
 while (NextNonComment->is(tok::comma))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-25 Thread Fred Grim via Phabricator via cfe-commits

feg208 added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:17805
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"

HazardyKnusperkeks wrote:
> Here is no alignment, maybe add another case with another line?
> 
> What happens if one line has the trailing comma and the next doesn't?
yesh this is pretty ugly. In the second case it crashes. This isn't going to be 
fixed for a bit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110481

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

[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-26 Thread Fred Grim via Phabricator via cfe-commits

feg208 added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:17805
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"

HazardyKnusperkeks wrote:
> feg208 wrote:
> > HazardyKnusperkeks wrote:
> > > Here is no alignment, maybe add another case with another line?
> > > 
> > > What happens if one line has the trailing comma and the next doesn't?
> > yesh this is pretty ugly. In the second case it crashes. This isn't going 
> > to be fixed for a bit
> What is that bit? Now we can still revert the original change for LLVM 13 (at 
> least I think so, since there are new RC Tags on GitHub), or can fix the 
> crash.
I think we should fix the crash since it’s only an issue with the dangling 
comma but if the consensus is we should revert then I can do that as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110481

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

[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-27 Thread Fred Grim via Phabricator via cfe-commits

feg208 added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:17805
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"

HazardyKnusperkeks wrote:
> feg208 wrote:
> > HazardyKnusperkeks wrote:
> > > feg208 wrote:
> > > > HazardyKnusperkeks wrote:
> > > > > Here is no alignment, maybe add another case with another line?
> > > > > 
> > > > > What happens if one line has the trailing comma and the next doesn't?
> > > > yesh this is pretty ugly. In the second case it crashes. This isn't 
> > > > going to be fixed for a bit
> > > What is that bit? Now we can still revert the original change for LLVM 13 
> > > (at least I think so, since there are new RC Tags on GitHub), or can fix 
> > > the crash.
> > I think we should fix the crash since it’s only an issue with the dangling 
> > comma but if the consensus is we should revert then I can do that as well
> Totally agree, could you please add the test cases? The ones that crash you 
> could comment out and fix afterwards.
> 
> I don't know how much time we have before the next version is released.
Let me see if I can fix the weirdness with the second test case but I'll time 
box it to two days so this doesn't drag out. I'd feel better about it not 
crashing just because we added an additional row.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110481

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

[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-28 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 375544.
feg208 added a comment.

Gets rid of the crash for the second added case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110481

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17800,13 +17800,25 @@
 TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) {
   auto Style = getLLVMStyle();
   Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  /* FIXME: This case gets misformatted.
+  verifyFormat("auto foo = Items{\n"
+   "Section{0, bar(), },\n"
+   "Section{1, boo()  }\n"
+   "};\n",
+   Style);
+  */
+  verifyFormat("auto foo = Items{\n"
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"
+   "};\n",
+   Style);
   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"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1146,14 +1146,15 @@
   } else if (C.Tok->is(tok::comma)) {
 if (!Cells.empty())
   Cells.back().EndIndex = i;
-Cell++;
+if (C.Tok->getNextNonComment()->isNot(tok::r_brace)) // dangling comma
+  ++Cell;
   }
 } else if (Depth == 1) {
   if (C.Tok == MatchingParen) {
 if (!Cells.empty())
   Cells.back().EndIndex = i;
 Cells.push_back(CellDescription{i, ++Cell, i + 1, false, nullptr});
-CellCount = Cell + 1;
+CellCount = C.Tok->Previous->isNot(tok::comma) ? Cell + 1 : Cell;
 // Go to the next non-comment and ensure there is a break in front
 const auto *NextNonComment = C.Tok->getNextNonComment();
 while (NextNonComment->is(tok::comma))
@@ -1190,6 +1191,17 @@
 // So if we split a line previously and the tail line + this token is
 // less then the column limit we remove the split here and just put
 // the column start at a space past the comma
+//
+// FIXME This if branch covers the cases where the column is not
+// the first column. This leads to weird pathologies like the 
formatting
+// auto foo = Items{
+// Section{
+// 0, bar(),
+// }
+// };
+// Well if it doesn't lead to that it's indicative that the line
+// breaking should be revisited. Unfortunately alot of other options
+// interact with this
 auto j = i - 1;
 if ((j - 1) > Start && Changes[j].Tok->is(tok::comma) &&
 Changes[j - 1].NewlinesBefore > 0) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17800,13 +17800,25 @@
 TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) {
   auto Style = getLLVMStyle();
   Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  /* FIXME: This case gets misformatted.
+  verifyFormat("auto foo = Items{\n"
+   "Section{0, bar(), },\n"
+   "Section{1, boo()  }\n"
+   "};\n",
+   Style);
+  */
+  verifyFormat("auto foo = Items{\n"
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"
+   "};\n",
+   Style);
   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"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1146,14 +1146,15 @@
   } else if (C.Tok->is(tok::comma)) {
 if (!Cells.empty())
   Cells.back().EndIndex = i;
-Cell++;
+if (C.Tok->getNextNonComment()->isNot(tok::r_brace)) // dangling comma
+

[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-28 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

So I added in the second case but commented it out as it gets misformatted. 
However it doesn't assert which was the alarming outcome. I have one other bug 
to clean up here and I am going to open a bug to fix the line breaking after 
initial { in certain cases since the code there needs to be revisited. I am 
doing formatting in the info gathering loop where really I should add all that 
in the back end. But thats a bigger change that I'd like to do outside of 
mitigating this users crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110481

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

[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-28 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

By one other bug I mean an entirely separate bug that I will handle in a 
separate PR. Sorry if that was confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110481

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

[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-28 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

Absolutely in agreement


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110481

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

[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-28 Thread Fred Grim via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa36227cb2b6a: fixes bug #51926 where dangling comma caused 
overrun (authored by feg208).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110481

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17800,13 +17800,25 @@
 TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) {
   auto Style = getLLVMStyle();
   Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  /* FIXME: This case gets misformatted.
+  verifyFormat("auto foo = Items{\n"
+   "Section{0, bar(), },\n"
+   "Section{1, boo()  }\n"
+   "};\n",
+   Style);
+  */
+  verifyFormat("auto foo = Items{\n"
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"
+   "};\n",
+   Style);
   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"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1146,14 +1146,15 @@
   } else if (C.Tok->is(tok::comma)) {
 if (!Cells.empty())
   Cells.back().EndIndex = i;
-Cell++;
+if (C.Tok->getNextNonComment()->isNot(tok::r_brace)) // dangling comma
+  ++Cell;
   }
 } else if (Depth == 1) {
   if (C.Tok == MatchingParen) {
 if (!Cells.empty())
   Cells.back().EndIndex = i;
 Cells.push_back(CellDescription{i, ++Cell, i + 1, false, nullptr});
-CellCount = Cell + 1;
+CellCount = C.Tok->Previous->isNot(tok::comma) ? Cell + 1 : Cell;
 // Go to the next non-comment and ensure there is a break in front
 const auto *NextNonComment = C.Tok->getNextNonComment();
 while (NextNonComment->is(tok::comma))
@@ -1190,6 +1191,17 @@
 // So if we split a line previously and the tail line + this token is
 // less then the column limit we remove the split here and just put
 // the column start at a space past the comma
+//
+// FIXME This if branch covers the cases where the column is not
+// the first column. This leads to weird pathologies like the 
formatting
+// auto foo = Items{
+// Section{
+// 0, bar(),
+// }
+// };
+// Well if it doesn't lead to that it's indicative that the line
+// breaking should be revisited. Unfortunately alot of other options
+// interact with this
 auto j = i - 1;
 if ((j - 1) > Start && Changes[j].Tok->is(tok::comma) &&
 Changes[j - 1].NewlinesBefore > 0) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17800,13 +17800,25 @@
 TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) {
   auto Style = getLLVMStyle();
   Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  /* FIXME: This case gets misformatted.
+  verifyFormat("auto foo = Items{\n"
+   "Section{0, bar(), },\n"
+   "Section{1, boo()  }\n"
+   "};\n",
+   Style);
+  */
+  verifyFormat("auto foo = Items{\n"
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"
+   "};\n",
+   Style);
   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"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1146,14 +1146,15 @@
   } else if (C.Tok->is(tok::comma)) {
 if (!Cells.empty())
   Cells.b

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

2021-06-03 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 349589.
feg208 marked 2 inline comments as done.
feg208 added a comment.

Grabs up some review comments and adds lit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
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{\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 "
+

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

2021-06-03 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

Got both of these




Comment at: clang/lib/Format/TokenAnnotator.cpp:737-740
+const auto End = Contexts.rbegin() + 2;
+auto Last = Contexts.rbegin();
+unsigned Depth = 0;
+for (; Last != End; Last = std::next(Last)) {

HazardyKnusperkeks wrote:
> I actually meant so. Because now this is even safe if the iterators are not 
> random access anymore in the future.
Oh I see. I sort of wondered about that but it didn't seem like a hill to die 
on.



Comment at: clang/lib/Format/WhitespaceManager.cpp:1136-1137
+auto j = i - 1;
+for (; j > 0 && Changes[j].NewlinesBefore == 0; --j) {
+}
+EndSpaces = Changes[j].Spaces;

HazardyKnusperkeks wrote:
> I don't know how to do this in LLVM style, but maybe so?
So long as clang-format doesn't complain I am fine either way


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-06-03 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 349728.
feg208 added a comment.

Rebased against main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16601,6 +16601,229 @@
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{\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 multip

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

2021-06-03 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 349730.
feg208 added a comment.

Had some invalid code in the lit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16601,6 +16601,229 @@
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{\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

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

2021-06-07 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-06-08 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-06-08 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 350746.
feg208 added a comment.

This alters the array alignment to allow for left alignment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer-left.cpp
  clang/test/Format/struct-array-initializer-right.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  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{\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"
+  "};",
+  fo

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

2021-06-08 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 350774.
feg208 added a comment.

Fixes from clang-tidy checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer-left.cpp
  clang/test/Format/struct-array-initializer-right.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  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{\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, \

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 350910.
feg208 marked 4 inline comments as done.
feg208 added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer-left.cpp
  clang/test/Format/struct-array-initializer-right.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  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{\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,  "
+  "

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

Got some changes. The failing test is an open issue




Comment at: clang/docs/ClangFormatStyleOptions.rst:216-221
+struct test demo[] =
+{
+{56,23, "hello"},
+{-1, 93463, "world"},
+{ 7, 5,"!!"}
+};

curdeius wrote:
> Don't forget to re-generate .rst.
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 350920.
feg208 added a comment.

Fixes the busted test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  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{\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

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 350924.
feg208 added a comment.

Ick. Missed a semi colon in the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  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{\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 "
+ "

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 350974.
feg208 added a comment.

Regenerated the mangled ClangFormatStyle.rst file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  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{\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 "
+

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits

feg208 marked an inline comment as done.
feg208 added a comment.

addressed the remaining review comment




Comment at: clang/docs/ClangFormatStyleOptions.rst:216-221
+struct test demo[] =
+{
+{56,23, "hello"},
+{-1, 93463, "world"},
+{ 7, 5,"!!"}
+};

curdeius wrote:
> feg208 wrote:
> > curdeius wrote:
> > > Don't forget to re-generate .rst.
> > 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?
> 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).
Got it. Sorry for the confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits

feg208 marked an inline comment as done.
feg208 added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 350984.
feg208 marked 11 inline comments as done.
feg208 added a comment.

Rolls up the remaining review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16602,6 +16602,407 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  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{\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 w

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

2021-06-09 Thread Fred Grim via Phabricator via cfe-commits

feg208 marked 3 inline comments as done.
feg208 added a comment.

All the review comments are addressed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-06-10 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-06-10 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

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 fg...@apple.com

Thanks much


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D104242: Removes an unused variable and alters a lit test to simplify and avoid a buildbot error

2021-06-14 Thread Fred Grim via Phabricator via cfe-commits

feg208 created this revision.
feg208 added reviewers: HazardyKnusperkeks, Bigcheese.
feg208 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104242

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/test/Format/struct-array-initializer.cpp


Index: clang/test/Format/struct-array-initializer.cpp
===
--- clang/test/Format/struct-array-initializer.cpp
+++ clang/test/Format/struct-array-initializer.cpp
@@ -1,9 +1,5 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Right}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Left}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Right}" %s | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Left}" %s | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
 struct test {
   int a;
   int b;
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1068,9 +1068,6 @@
 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces;
   ++CellIter;
   for (auto i = 1U; i < CellDescs.CellCount; i++, ++CellIter) {
-unsigned NetWidth = 0U;
-if (isSplitCell(*CellIter))
-  NetWidth = getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
 auto MaxNetWidth = getMaximumNetWidth(
 Cells.begin(), CellIter, CellDescs.InitialSpaces, CellDescs.CellCount);
 auto ThisNetWidth =


Index: clang/test/Format/struct-array-initializer.cpp
===
--- clang/test/Format/struct-array-initializer.cpp
+++ clang/test/Format/struct-array-initializer.cpp
@@ -1,9 +1,5 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Right}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Left}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Right}" %s | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Left}" %s | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
 struct test {
   int a;
   int b;
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1068,9 +1068,6 @@
 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces;
   ++CellIter;
   for (auto i = 1U; i < CellDescs.CellCount; i++, ++CellIter) {
-unsigned NetWidth = 0U;
-if (isSplitCell(*CellIter))
-  NetWidth = getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
 auto MaxNetWidth = getMaximumNetWidth(
 Cells.begin(), CellIter, CellDescs.InitialSpaces, CellDescs.CellCount);
 auto ThisNetWidth =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104242: Removes an unused variable and alters a lit test to simplify and avoid a buildbot error

2021-06-14 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

In D104242#2817815 , 
@HazardyKnusperkeks wrote:

> Could you split this into two diffs? Or are those two changes in any way 
> related?

I can if you'd like. They are related in the sense that the dead code yielded 
warnings in the buildbot logs that necessitated this change set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104242

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

[PATCH] D104242: Removes an unused variable and alters a lit test to simplify and avoid a buildbot error

2021-06-14 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 351982.
feg208 added a comment.

Looks like the dead code is removed in main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104242

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


Index: clang/test/Format/struct-array-initializer.cpp
===
--- clang/test/Format/struct-array-initializer.cpp
+++ clang/test/Format/struct-array-initializer.cpp
@@ -1,9 +1,5 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Right}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Left}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Right}" %s | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Left}" %s | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
 struct test {
   int a;
   int b;


Index: clang/test/Format/struct-array-initializer.cpp
===
--- clang/test/Format/struct-array-initializer.cpp
+++ clang/test/Format/struct-array-initializer.cpp
@@ -1,9 +1,5 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Right}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Left}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Right}" %s | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Left}" %s | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
 struct test {
   int a;
   int b;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2021-06-14 Thread Fred Grim via Phabricator via cfe-commits

feg208 added inline comments.



Comment at: clang/test/Format/struct-array-initializer.cpp:5
+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Left}" %s \
+// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK2 %s

vitalybuka wrote:
> 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
> --
> 
> ```
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D104242: Alters a lit test to simplify and avoid a buildbot error

2021-06-14 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

I don’t have commit rights currently. Could I get someone to land this on my 
behalf?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104242

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

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

2021-06-15 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D104295: removes redundant test

2021-06-15 Thread Fred Grim via Phabricator via cfe-commits

feg208 created this revision.
feg208 added reviewers: MyDeveloperDay, HazardyKnusperkeks, vitalybuka.
feg208 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This lit test is causing intermittent failures on buildbots. It isn't testing 
anything
the unittests don't.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104295

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


Index: clang/test/Format/struct-array-initializer.cpp
===
--- clang/test/Format/struct-array-initializer.cpp
+++ /dev/null
@@ -1,60 +0,0 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Right}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Left}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
-struct test {
-  int a;
-  int b;
-  const char *c;
-};
-
-struct toast {
-  int a;
-  const char *b;
-  int c;
-  float d;
-};
-
-void f() {
-  struct test demo[] = {{56, 23, "hello"}, {-1, 93463, "world"}, {7, 5, "!!"}};
-  // CHECK1: {{^[[:space:]]{2}struct test demo\[\] = \{$}}
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{56,23, "hello"},
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{-1, 93463, "world"},
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{ 7, 5,"!!"}
-  // CHECK1-NEXT: {{^[[:space:]]{2}\};$}}
-}
-
-void g() {
-  struct toast demo[] = {
-  {56, "hello world I have some things to say", 30, 4.2},
-  {93463, "those things are really comments", 1, 3.1},
-  {7, "about a wide range of topics", 789, .112233}};
-  // CHECK1: {{^[[:space:]]{2}struct toast demo\[\] = \{$}}
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{   56, "hello world I have some things 
to say",  30, 4.2},
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{93463,  "those things are really 
comments",   1, 3.1},
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{7,  "about a wide range of 
topics", 789, .112233}
-  // CHECK1-NEXT: {{^[[:space:]]{2}\};$}}
-}
-
-void h() {
-  struct test demo[] = {{56, 23, "hello"}, {-1, 93463, "world"}, {7, 5, "!!"}};
-  // CHECK2: {{^[[:space:]]{2}struct test demo\[\] = \{$}}
-  // CHECK2-NEXT: {{([[:space:]]{4})}}{56, 23,"hello"},
-  // CHECK2-NEXT: {{([[:space:]]{4})}}{-1, 93463, "world"},
-  // CHECK2-NEXT: {{([[:space:]]{4})}}{7,  5, "!!"   }
-  // CHECK2-NEXT: {{^[[:space:]]{2}\};$}}
-}
-
-void i() {
-  struct toast demo[] = {
-  {56, "hello world I have some things to say", 30, 4.2},
-  {93463, "those things are really comments", 1, 3.1},
-  {7, "about a wide range of topics", 789, .112233}};
-  // CHECK2: {{^[[:space:]]{2}struct toast demo\[\] = \{$}}
-  // CHECK2-NEXT: {{([[:space:]]{4})}}{56,"hello world I have some things 
to say", 30,  4.2},
-  // CHECK2-NEXT: {{([[:space:]]{4})}}{93463, "those things are really 
comments",  1,   3.1},
-  // CHECK2-NEXT: {{([[:space:]]{4})}}{7, "about a wide range of topics",  
789, .112233}
-  // CHECK2-NEXT: {{^[[:space:]]{2}\};$}}
-}


Index: clang/test/Format/struct-array-initializer.cpp
===
--- clang/test/Format/struct-array-initializer.cpp
+++ /dev/null
@@ -1,60 +0,0 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Right}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Left}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
-struct test {
-  int a;
-  int b;
-  const char *c;
-};
-
-struct toast {
-  int a;
-  const char *b;
-  int c;
-  float d;
-};
-
-void f() {
-  struct test demo[] = {{56, 23, "hello"}, {-1, 93463, "world"}, {7, 5, "!!"}};
-  // CHECK1: {{^[[:space:]]{2}struct test demo\[\] = \{$}}
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{56,23, "hello"},
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{-1, 93463, "world"},
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{ 7, 5,"!!"}
-  // CHECK1-NEXT: {{^[[:space:]]{2}\};$}}
-}
-
-void g() {
-  struct toast demo[] = {
-  {56, "hello world I have some things to say", 30, 4.2},
-  {93463, "those things are really comments", 1, 3.1},
-  {7, "about a wide range of topics", 789, .112233}};
-  // CHECK1: {{^[[:space:]]{2}struct toast demo\[\] = \{$}}
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{   56, "hello world I have some things to say",  30, 4.2},
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{93463,  "those things are really comments",   1, 3.1},
-  // CHECK1-NEXT: {{([[:space:]]{4})}}{7,  "about a wide range of topics", 789, .112233}
-  // CHECK1-NEXT: {{^[[:space:]]{2}\};$}}
-}
-
-void h() {
-  str

[PATCH] D104242: Alters a lit test to simplify and avoid a buildbot error

2021-06-15 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

I put up a separate commit that just removes the test. I think this would 
resolve the builder issues but I don't think its worth even trying. We should 
abandon this (I think)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104242

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

[PATCH] D104295: removes redundant test

2021-06-15 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

If I could get someone to land this commit on my behalf I'd be obliged. I am 
working on the getting commit rights since that seems to be a pretty 
significant impediment to developing in llvm land but as of now I can't land 
this myself.

name: Fred Grim
email: fg...@apple.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104295

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

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

2021-06-15 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

2022-03-06 Thread Fred Grim via Phabricator via cfe-commits

feg208 accepted this revision.
feg208 added a comment.

I have been watching the issues pile up around this but work has prevented me 
from focusing on them. This seems like a reasonable compromise but the note of 
disabling this feature is also reasonable. Given where it had to be placed in 
the formatting flow I suspect it will be challenging to resolve most of the 
troublesome corner cases. I did investigate one bug I picked up but ran into 
the problems you described earlier and was going down the same compromise path.


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

https://reviews.llvm.org/D121069

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

2022-03-06 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

In D121069#3362450 , @MyDeveloperDay 
wrote:

> @feg208 I could do with some clarity on the algorithm, am I correct in 
> thinking it requires that the first row, to contain at least the maximum 
> number of columns for the rest of the array. I sort of noticed it was fine if 
> the first row was larger
>
>   {
>   {1,2,3 }
>   {4,5}
>   {6}
>   }

Honestly it really is best with the square option. The assumption is an array 
of simple initializers though default args to constructors complicate that in 
the case of an array of single constructors. I think that you are seeing it be 
okay with the maximum as the first row is just an accident of the 
implementation. The direction I was heading in in the bug I picked up was to 
start restricting the set of things it would consider as array of initializers. 
Does that answer your question?

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

https://reviews.llvm.org/D121069

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

[PATCH] D101868: Adds a formatter for aligning arrays of structs

2021-05-04 Thread Fred Grim via Phabricator via cfe-commits

feg208 created this revision.
feg208 added reviewers: tinloaf, djasper, klimek, curdeius.
feg208 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16347,6 +16347,29 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructuresInit) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructuresInit = true;
+  verifyFormat("struct test demo[] = {\n"
+   "{56, 23,\"hello\" },\n"
+   "{-1, 93463, \"world\" },\n"
+   "{7,  5, \"!!\"}\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);
+}
+
 TEST_F(FormatTest, UnderstandsPragmas) {
   verifyFormat("#pragma omp reduction(| : var)");
   verifyFormat("#pragma omp reduction(+ : var)");
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -10,7 +10,9 @@
 #include "NamespaceEndCommentsFixer.h"
 #include "WhitespaceManager.h"
 #include "llvm/Support/Debug.h"
+#include 
 #include 
+#include 
 
 #define DEBUG_TYPE "format-formatter"
 
@@ -26,6 +28,62 @@
  NextNext && NextNext->is(tok::l_brace);
 }
 
+// The notion here is that we walk through the annotated line looking for
+// things like static initialization of arrays and flag them
+bool isArrayOfStructuresInit(const AnnotatedLine &Line) {
+  if (!Line.MustBeDeclaration)
+return false;
+  const auto *CurrentToken = Line.First;
+  enum class DetectAsiFsm {
+null,
+in_struct_decl,
+in_type_decl,
+in_var_name_decl,
+in_bracket_decl,
+finished_bracket_decl,
+outer_l_brace
+  };
+  DetectAsiFsm AsiFsm{DetectAsiFsm::null};
+  while (CurrentToken != Line.Last && CurrentToken != nullptr) {
+if (CurrentToken->is(tok::kw_struct)) {
+  if (AsiFsm != DetectAsiFsm::null)
+return false;
+  AsiFsm = DetectAsiFsm::in_struct_decl;
+} else if (CurrentToken->is(tok::identifier)) {
+  switch (AsiFsm) {
+  case DetectAsiFsm::null:
+[[clang::fallthrough]];
+  case DetectAsiFsm::in_struct_decl:
+AsiFsm = DetectAsiFsm::in_type_decl;
+break;
+  case DetectAsiFsm::in_type_decl:
+AsiFsm = DetectAsiFsm::in_var_name_decl;
+break;
+  default:
+return false;
+  }
+} else if (CurrentToken->is(tok::l_square)) {
+  if (AsiFsm != DetectAsiFsm::in_var_name_decl)
+return false;
+  AsiFsm = DetectAsiFsm::in_bracket_decl;
+} else if (CurrentToken->is(tok::numeric_constant)) {
+  if (AsiFsm != DetectAsiFsm::in_bracket_decl)
+return false;
+} else if (CurrentToken->is(tok::r_square)) {
+  if (AsiFsm != DetectAsiFsm::in_bracket_decl)
+return false;
+  AsiFsm = DetectAsiFsm::finished_bracket_decl;
+} else if (CurrentToken->is(tok::l_brace)) {
+  if (AsiFsm == DetectAsiFsm::finished_bracket_decl)
+AsiFsm = DetectAsiFsm::outer_l_brace;
+  else
+return AsiFsm == DetectAsiFsm::outer_l_brace;
+}
+CurrentToken = CurrentToken->getNextNonComment();
+  }
+  return false;
+}
+
 /// Tracks the indent level of \c AnnotatedLines across levels.
 ///
 /// \c nextLine must be called for each \c AnnotatedLine, after which \c
@@ -1101,6 +1159,176 @@
   llvm::SpecificBumpPtrAllocator Allocator;
 };
 
+/// Breaks a static array of struct initializers into regular
+/// columns
+class AlignedArrayOfStructuresInitLineFormatter : public LineFormatter {
+  struct FormatArgs {
+LineState &State;
+unsigned &Penalty;
+unsigned FirstIndent;
+unsigned FirstStartColumn;
+bool DryRun;
+  };
+
+public:
+  AlignedArrayOfStructuresInitLineFormatter(
+  ContinuationIndenter *Indenter, WhitespaceManager *Whitespaces,
+  const FormatStyle &Style, UnwrappedLineFormatter *BlockFormatter)
+  : LineFormatter

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

2021-05-06 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 343534.
feg208 marked 8 inline comments as done.
feg208 added a comment.

This addresses most of the review comments. There remain a few tests to add


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15,6 +15,8 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "gtest/gtest.h"
 
+#include 
+
 #define DEBUG_TYPE "format-test"
 
 using clang::tooling::ReplacementTest;
@@ -16347,6 +16349,94 @@
getLLVMStyle());
 }
 
+template 
+auto createStylesImpl(
+const std::array &ACSValues,
+const std::array &Limits) {
+  std::array Styles;
+  auto StyleIter = Styles.begin();
+  for (const auto &OuterACS : ACSValues) {
+for (const auto &InnerACS : ACSValues) {
+  for (const auto &LimitValue : Limits) {
+auto &StyleValue = *(StyleIter);
+StyleValue = getLLVMStyle();
+StyleValue.AlignArrayOfStructures = true;
+StyleValue.AlignConsecutiveAssignments = OuterACS;
+StyleValue.AlignConsecutiveDeclarations = InnerACS;
+StyleValue.ColumnLimit = LimitValue;
+++StyleIter;
+  }
+}
+  }
+  return Styles;
+}
+
+auto createStyles() {
+  constexpr static auto N = 5;
+  std::array ACSValues{
+  {FormatStyle::AlignConsecutiveStyle::ACS_None,
+   FormatStyle::AlignConsecutiveStyle::ACS_Consecutive,
+   FormatStyle::AlignConsecutiveStyle::ACS_AcrossEmptyLines,
+   FormatStyle::AlignConsecutiveStyle::ACS_AcrossComments,
+   FormatStyle::AlignConsecutiveStyle::ACS_AcrossEmptyLinesAndComments}};
+  std::array LimitValues{{0, 80}};
+  return createStylesImpl(ACSValues, LimitValues);
+}
+
+TEST_F(FormatTest, CatchAlignArrayOfStructures) {
+  const static auto Styles = createStyles();
+  for (const auto &Style : Styles) {
+verifyFormat("struct test demo[] = {\n"
+ "{56, 23,\"hello\" },\n"
+ "{-1, 93463, \"world\" },\n"
+ "{7,  5, \"!!\"}\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{\n"
+ "test{56, 23,\"hello\" },\n"
+ "test{-1, 93463, \"world\" },\n"
+ "test{7,  5, \"!!\"},\n"
+ "};\n",
+ Style);
+  }
+
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = true;
+  verifyFormat(
+  "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",
+  Style);
+}
+
 TEST_F(FormatTest, UnderstandsPragmas) {
   verifyFormat("#pragma omp reduction(| : var)");
   verifyFormat("#pragma omp reduction(+ : var)");
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -10,7 +10,11 @@
 #include "NamespaceEndCommentsFixer.h"
 #include "WhitespaceManager.h"
 #include "llvm/Support/Debug.h"
+#include 
+#include 
 #include 
+#include 
+#include 
 
 #define DEBUG_TYPE "format-formatter"
 
@@ -26,6 +30,54 @@
  NextNext && NextNext->is(tok::l_brace);
 }
 
+// The notion here is that we walk throug

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

2021-05-06 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

To answer the question of why I think this is different then other alignment 
optionsIt seems to me that each alignment option emphasizes a specific 
thing, be it macros, bitfields, or (maybe closer in spirit) more simple 
declarations and assignments. I think this case is unique and not currently 
addressed by any of the existing alignment mechanisms.




Comment at: clang/include/clang/Format/Format.h:93
 
+  /// if ``true``, when using static initialization for an array of structs
+  /// aligns the fields into columns

curdeius wrote:
> Why static?
I was quoting from an internal ticket but it doesn't make sense. I removed it



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:32
+// The notion here is that we walk through the annotated line looking for
+// things like static initialization of arrays and flag them
+bool isArrayOfStructuresInit(const AnnotatedLine &Line) {

HazardyKnusperkeks wrote:
> Dot at the end. :)
One of these days I'll learn the English language. Then watch out!



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:37
+  const auto *CurrentToken = Line.First;
+  enum class DetectAsiFsm {
+null,

HazardyKnusperkeks wrote:
> What stands AsiFsm for?
Aligned Structure Initializer Finite State Machine but honestly this entire 
flow was misguided as the suggested tests pointed out. I switched it to 
something that fits (I think) a bit better with a wider array of possible inputs



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1232
+  } else if (CurrentToken->is(tok::r_brace)) {
+Depth--;
+  }

HazardyKnusperkeks wrote:
> Why postfix, especially if using prefix in the `if`?
just an oversight. In hindsight it does look goofy ehh?



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1404
+ isArrayOfStructuresInit(TheLine));
+  if (AlignArrayOfStructuresInit) {
+Penalty += AlignedArrayOfStructuresInitLineFormatter(

HazardyKnusperkeks wrote:
> I'm not sure that you should go before the no column limit. In either case 
> you should have a test case for that.
I kept the ordering but I will admit to misgivings about it. None of the tests 
fail and it seems like it needs to be ordered in this way but as a newbie 
contributor to this project I don't feel confident in that assertion. Do the 
included tests cover it? What test that isn't currently there would better 
create issues?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-05-06 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

I have a few outstanding questions about additional testing




Comment at: clang/unittests/Format/FormatTest.cpp:16371
+   Style);
+}
+

HazardyKnusperkeks wrote:
> curdeius wrote:
> > I think we need more tests:
> > * with a comma after the last element of array
> > * with assignment and not only initialization?
> > * without `struct` keyword
> > * with short lines/long values to see how wrapping behaves
> >   * also, with multiple string literals in one struct, e.g. `"hello" 
> > "world"` (that will get splitted into multiple lines)
> > * with non-plain-array types (i.e. missing `[]`/`[3]`)
> > * interaction with other Align* options (e.g. 
> > `AlignConsecutiveAssignments`, two arrays one after another, 
> > `AlignConsecutiveDeclarations`).
> In addition to that I want to see:
> * How it behaves with a smaller column limit which would be violated if there 
> is no additional wrapping. There I also want one without `verifyFormat`.
> * With no column limit.
> * Maybe with a `const`, `inline` or something similar. (How about east 
> `const` vs. west `const`?)
I added

  - with a comma after the last element of array
  - with assignment and not only initialization?
  - without struct keyword
  - with short lines/long values to see how wrapping behaves (also multiple 
string literals) But I still should add another test I think
  - with non-plain-array types (i.e. missing []/[3])
  - interaction with other Align* options (e.g. AlignConsecutiveAssignments, 
two arrays one after another, AlignConsecutiveDeclarations). I will say that I 
did this in a brute force way. Maybe there is something else I should do here?
  - With no column limit.

I still need tests with east and west const and without verifyFormat. For the 
latter is there an example test I could look at? Idly scrolling through 
FormatTest.cpp it seems like most stuff uses verifyFormat? Or maybe I 
misunderstood the request?


















Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-05-07 Thread Fred Grim via Phabricator via cfe-commits

feg208 marked 4 inline comments as done.
feg208 added a comment.

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




Comment at: clang/unittests/Format/FormatTest.cpp:16352
 
+template 
+auto createStylesImpl(

curdeius wrote:
> HazardyKnusperkeks wrote:
> > This is a nice approach!
> > 
> > Could it be made constexpr? I'm not sure about C++14.
> 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.
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-05-07 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 343747.
feg208 added a comment.

Added more tests and addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15,6 +15,8 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "gtest/gtest.h"
 
+#include 
+
 #define DEBUG_TYPE "format-test"
 
 using clang::tooling::ReplacementTest;
@@ -16347,6 +16349,147 @@
getLLVMStyle());
 }
 
+template 
+auto createStylesImpl(
+const std::array &ACSValues,
+const std::array &Limits) {
+  std::array Styles;
+  auto StyleIter = Styles.begin();
+  for (const auto &OuterACS : ACSValues) {
+for (const auto &InnerACS : ACSValues) {
+  for (const auto &LimitValue : Limits) {
+auto &StyleValue = *(StyleIter);
+StyleValue = getLLVMStyle();
+StyleValue.AlignArrayOfStructures = true;
+StyleValue.AlignConsecutiveAssignments = OuterACS;
+StyleValue.AlignConsecutiveDeclarations = InnerACS;
+StyleValue.ColumnLimit = LimitValue;
+++StyleIter;
+  }
+}
+  }
+  return Styles;
+}
+
+auto createStyles() {
+  constexpr static auto N = 5;
+  std::array ACSValues{
+  {FormatStyle::AlignConsecutiveStyle::ACS_None,
+   FormatStyle::AlignConsecutiveStyle::ACS_Consecutive,
+   FormatStyle::AlignConsecutiveStyle::ACS_AcrossEmptyLines,
+   FormatStyle::AlignConsecutiveStyle::ACS_AcrossComments,
+   FormatStyle::AlignConsecutiveStyle::ACS_AcrossEmptyLinesAndComments}};
+  std::array LimitValues{{0, 80}};
+  return createStylesImpl(ACSValues, LimitValues);
+}
+
+TEST_F(FormatTest, CatchAlignArrayOfStructures) {
+  const static auto Styles = createStyles();
+  for (const auto &Style : Styles) {
+verifyFormat("struct test demo[] = {\n"
+ "{56, 23,\"hello\" },\n"
+ "{-1, 93463, \"world\" },\n"
+ "{7,  5, \"!!\"}\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{\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);
+  }
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = true;
+  verifyFormat(
+  "test demo[] = {\n"
+  "{56, 23,\"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"
+  "{7,  5, \"!!\""
+  "},\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, \"!!\"

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

2021-05-07 Thread Fred Grim via Phabricator via cfe-commits

feg208 updated this revision to Diff 343748.
feg208 marked an inline comment as done.
feg208 added a comment.

Oops missed a request


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15,6 +15,8 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "gtest/gtest.h"
 
+#include 
+
 #define DEBUG_TYPE "format-test"
 
 using clang::tooling::ReplacementTest;
@@ -16347,6 +16349,147 @@
getLLVMStyle());
 }
 
+template 
+auto createStylesImpl(
+const std::array &ACSValues,
+const std::array &Limits) {
+  std::array Styles;
+  auto StyleIter = Styles.begin();
+  for (const auto &OuterACS : ACSValues) {
+for (const auto &InnerACS : ACSValues) {
+  for (const auto &LimitValue : Limits) {
+auto &StyleValue = *(StyleIter);
+StyleValue = getLLVMStyle();
+StyleValue.AlignArrayOfStructures = true;
+StyleValue.AlignConsecutiveAssignments = OuterACS;
+StyleValue.AlignConsecutiveDeclarations = InnerACS;
+StyleValue.ColumnLimit = LimitValue;
+++StyleIter;
+  }
+}
+  }
+  return Styles;
+}
+
+auto createStyles() {
+  constexpr static auto N = 5;
+  std::array ACSValues{
+  {FormatStyle::AlignConsecutiveStyle::ACS_None,
+   FormatStyle::AlignConsecutiveStyle::ACS_Consecutive,
+   FormatStyle::AlignConsecutiveStyle::ACS_AcrossEmptyLines,
+   FormatStyle::AlignConsecutiveStyle::ACS_AcrossComments,
+   FormatStyle::AlignConsecutiveStyle::ACS_AcrossEmptyLinesAndComments}};
+  std::array LimitValues{{0, 80}};
+  return createStylesImpl(ACSValues, LimitValues);
+}
+
+TEST_F(FormatTest, CatchAlignArrayOfStructures) {
+  const static auto Styles = createStyles();
+  for (const auto &Style : Styles) {
+verifyFormat("struct test demo[] = {\n"
+ "{56, 23,\"hello\" },\n"
+ "{-1, 93463, \"world\" },\n"
+ "{7,  5, \"!!\"}\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{\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);
+  }
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = true;
+  verifyFormat(
+  "test demo[] = {\n"
+  "{56, 23,\"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"
+  "{7,  5, \"!!\""
+  "},\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, \"!!\"

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

2021-05-07 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

Missed a request in the release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-05-08 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

I still need to handle the 20 character case appropriately




Comment at: clang/unittests/Format/FormatTest.cpp:16465
+  "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"

HazardyKnusperkeks wrote:
> But this line is longer than 20! This test should fail, shouldn't it?
It should. When I use the command line invocation I get different output

```
#include 
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



Comment at: clang/unittests/Format/FormatTest.cpp:16352
 
+template 
+auto createStylesImpl(

HazardyKnusperkeks wrote:
> feg208 wrote:
> > curdeius wrote:
> > > HazardyKnusperkeks wrote:
> > > > This is a nice approach!
> > > > 
> > > > Could it be made constexpr? I'm not sure about C++14.
> > > 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.
> > 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.
> > 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.
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

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

2021-05-08 Thread Fred Grim via Phabricator via cfe-commits

feg208 added a comment.

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.

hmm 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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

70 matches

Mail list logo