[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist updated this revision to Diff 135744.
enyquist added a comment.
Rebased on current master. No functional changes, just a minor conflict where a
variable got re-named
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8933,8 +8933,104 @@
verifyFormat("a or_eq 8;", Spaces);
}
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AlignConsecutiveAssignments = true;
+ Style.AlignConsecutiveDeclarations = true;
+ Style.AlignConsecutiveMacros = false;
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Style);
+
+ Style.AlignConsecutiveMacros = false;
+ Style.ColumnLimit = 20;
+
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveAssignments = false;
verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -9124,6 +9220,7 @@
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveDeclarations = false;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -10237,6 +10334,7 @@
Style.Language = FormatStyle::LK_Cpp;
CHECK_PARSE_BOOL(AlignOperands);
CHECK_PARSE_BOOL(AlignTrailingComments);
+ CHECK_PARSE_BOOL(AlignConsecutiveMacros);
CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -170,6 +170,9 @@
/// \c EscapedNewlineColumn for the first tokens or token
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added a comment. @dtzWill thanks for the suggestion, I have submitted this change to the weekly review corner. Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added a comment. Looks fine to me, although I confess I did not build and run it because I don't have the time to set up the environment again, took a few hours last time I built from scratch (side note, if there's an easy way to speed up llvm/clang compilation up I'd love to hear it :) ). You didn't change the tests that I can see, so if those are still passing then it seems to be a fairly successful refactor. However I'm very much a clang newbie (just to be clear), whether this satisfies the original request for a refactor I cannot comment on. Thanks for taking this over-- I started this because I really needed the feature, but eventually gave up because we didn't need it anymore after a while, and after chasing it on here for nearly a year it just wasn't worth the effort anymore. However, if it turns out that this really was the only thing left need to merge it, while being slightly annoyed that I gave up on what turned out to be the *actual* final change, it will still be nice to finally have this in clang-format :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28462/new/ https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added a comment. @smilewithani @lassi.niemisto @mewmew feel free to take a stab at it Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28462/new/ https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added a comment. @klimek fair point. To be honest, I've pretty much lost interest / momentum on this feature, I very much doubt I will ever go back and re-implement from scratch as you suggest. Not meaning to sound rude, I just don't want to waste anyone's time who is waiting for this (seems there are a few). Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added a comment. @klimek having gotten that out of the way, I do occasionally drink too much and have sudden urges to re-implement things from scratch. Close it if you need to, since I can't commit to anything, but it it happens to be still open on one of those nights, who knows, maybe I'll end up doing it :) Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added a comment. As far as I know, there are no updates required from me for this pull request-- I rebased on the main trunk recently, and will do it again tonight to be sure. So it should be compiling/working just fine. I believe it is just awaiting final approval from somebody. Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist created this revision.
enyquist added reviewers: djasper, sylvestre.ledru.
enyquist added a subscriber: cfe-commits.
enyquist set the repository for this revision to rL LLVM.
enyquist added a project: clang-c.
Herald added a subscriber: klimek.
This option behaves similarly to AlignConsecutiveDeclarations and
AlignConsecutiveAssignments, aligning the assignment of C/C++ preprocessor
macros on consecutive lines
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8514,8 +8514,78 @@
verifyFormat("a or_eq 8;", Spaces);
}
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+ FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveAssignments = true;
+ Alignment.AlignConsecutiveDeclarations = true;
+ Alignment.AlignConsecutiveMacros = false;
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Alignment);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Alignment);
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ Alignment.AlignConsecutiveMacros = true;
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Alignment);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Alignment);
+
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Alignment);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveAssignments = false;
verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -8656,7 +8726,10 @@
"int j = 2;",
Alignment);
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
" auto i = 0;\n"
" return 0;\n"
"};\n"
@@ -8692,6 +8765,7 @@
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveDeclarations = false;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -8851,7 +8925,10 @@
Alignment);
Alignment.AlignConsecutiveAssignments = true;
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
" auto ii = 0;\n"
" float j = 0;\n"
" return 0;\n"
@@ -9576,6 +9653,7 @@
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
CHECK_PARSE_BOOL(AlignOperands);
CHECK_PARSE_BOOL(AlignTrailingComments);
+ CHECK_PARSE_BOOL(AlignConsecutiveMacros);
CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Ind
[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
enyquist added a comment. Hey bmharper :) I've got a review open that conflicts with this one, just having a look to see what I'll need to refactor (https://reviews.llvm.org/D28462). In fact, I have a question-- the conflict is specifically in WhitespaceManager.cpp. Since I needed to detect PP macros containing changes in scope depth (code blocks surrounded by curly braces, macro parameter lists, etc), I was having the same problem as you-- AlignTokens was bailing out whenever the scope depth changed. In my case, I just added a new parameter to AlignTokens, MaxNestingLevelIncrease, indicating how much the level can increase before we stop alignment, making the allowable scope-depth configurable. For example, calling AlignTokens with this flag set to 2 will cause alignment to continue up until we increase scope by two levels. Now, my question- from what I can tell of your changes, it looks like my code can actually be simpler when this gets merged. The state of AlignTokens will be maintained across changing scope depths, and I won't need to modify AlignTokens so that it can survive something like "#define foo(x) ((x * 2) + 2)". Is this correct? https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
enyquist added a comment. Well, your patch is here for me to try, and it looks like it's been accepted. So I guess I should just pull my finger out and try it :) Thanks for your response-- I'll let you know if I come across any issues. https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added a comment. @sylvestre.ledru No, unfortunately not. My apologies, I've been taken up with work mostly. I will make a marked attempt to do it this weekend :) Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist updated this revision to Diff 93092.
enyquist added a comment.
Option implemented completely in WhitespaceManager.cpp, so no overhead if the
option isn't used. Also some minor style fixes.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7518,8 +7518,78 @@
verifyFormat("a or_eq 8;", Spaces);
}
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+ FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveAssignments = true;
+ Alignment.AlignConsecutiveDeclarations = true;
+ Alignment.AlignConsecutiveMacros = false;
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Alignment);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Alignment);
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ Alignment.AlignConsecutiveMacros = true;
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Alignment);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Alignment);
+
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Alignment);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveAssignments = false;
verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -7660,7 +7730,10 @@
"int j = 2;",
Alignment);
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
" auto i = 0;\n"
" return 0;\n"
"};\n"
@@ -7702,6 +7775,7 @@
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveDeclarations = false;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -7933,7 +8007,10 @@
Alignment);
Alignment.AlignConsecutiveAssignments = true;
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
" auto ii = 0;\n"
" float j = 0;\n"
" return 0;\n"
@@ -8659,6 +8736,7 @@
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
CHECK_PARSE_BOOL(AlignOperands);
CHECK_PARSE_BOOL(AlignTrailingComments);
+ CHECK_PARSE_BOOL(AlignConsecutiveMacros);
CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -165,6 +165,9 @@
/// \c EscapedNewlineColumn for the first tokens or token parts in a line.
void calcul
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist marked 8 inline comments as done.
enyquist added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:287
SmallVector
&Changes,
+bool ConsiderScope, bool ConsiderCommas,
unsigned StartAt) {
djasper wrote:
> I don't find it intuitive what "consider" means in this case. Can you add a
> comment? Are the two parameters ever set independently? If not, I'd prefer to
> keep one parameter for now as we have test coverage only for that.
Currently, they are never set independently, no. I will combine them and add an
explanatory comment.
Comment at: lib/Format/WhitespaceManager.cpp:413
+
+ while (Param && !Param->is(tok::l_paren)) {
+if (!Param->is(tok::identifier) && !Param->is(tok::comma))
djasper wrote:
> I think you should be able to use Current.MatchingParen here.
Hmm, I couldn't make this work... I just replaced this line with
while (Param && Param != Current->MatchingParen)
But it must not be doing what I think it's doing, since it made some tests fail.
Mind you, my C-brain might be taking over here, please let me know if I'm using
MatchingParen incorrectly
Comment at: unittests/Format/FormatTest.cpp:7733
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
djasper wrote:
> Why this change? This seems to be tested above.
I wanted to ensure this option didn't break other alignment options while I was
developing-- this is a remnant. I will remove it.
Comment at: unittests/Format/FormatTest.cpp:8010
Alignment.AlignConsecutiveAssignments = true;
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
djasper wrote:
> Same here?
Yes.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist updated this revision to Diff 93204.
enyquist marked 3 inline comments as done.
enyquist added a comment.
Addressed all comments, except for the one about FormatToken.MatchingParen (see
reply comment)
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7518,8 +7518,104 @@
verifyFormat("a or_eq 8;", Spaces);
}
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AlignConsecutiveAssignments = true;
+ Style.AlignConsecutiveDeclarations = true;
+ Style.AlignConsecutiveMacros = false;
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Style);
+
+ Style.AlignConsecutiveMacros = false;
+ Style.ColumnLimit = 20;
+
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveAssignments = false;
verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -7702,6 +7798,7 @@
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveDeclarations = false;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -8659,6 +8756,7 @@
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
CHECK_PARSE_BOOL(AlignOperands);
CHECK_PARSE_BOOL(AlignTrailingComments);
+ CHECK_PARSE_BOOL(AlignConsecutiveMacros);
CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -165,6 +165,9 @@
/// \c EscapedNewlineColumn for the first
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:413
+
+ while (Param && !Param->is(tok::l_paren)) {
+if (!Param->is(tok::identifier) && !Param->is(tok::comma))
djasper wrote:
> enyquist wrote:
> > djasper wrote:
> > > I think you should be able to use Current.MatchingParen here.
> > Hmm, I couldn't make this work... I just replaced this line with
> >
> > while (Param && Param != Current->MatchingParen)
> >
> > But it must not be doing what I think it's doing, since it made some tests
> > fail.
> > Mind you, my C-brain might be taking over here, please let me know if I'm
> > using MatchingParen incorrectly
> You shouldn't need a while loop. Just:
>
> if (!Current->MatchingParen || !Current->MatchingParen->Previous)
> return false;
> Param = Param->MatchingParen->Previous;
>
> And with that, I think you can simplify all these methods a bit:
>
> static bool endsPPIdentifier(const FormatToken *Current) {
> if (!Current->Next || Current->Next->SpacesRequiredBefore == 0)
> return false;
>
> // If Current is a "(", skip past the parameter list.
> if (Current->is(tok::r_paren)) {
> if (!Current->MatchingParen || !Current->MatchingParen->Previous)
> return false;
> Current = Current->MatchingParen->Previous;
> }
>
> const FormatToken *Keyword = Current->Previous;
> if (!Keyword || !Keyword->is(tok::pp_define))
> return false;
>
> return Current->is(tok::identifier);
> }
>
> Does that work? If so, I'd even suggest inlining this code directly into the
> lambda instead of pulling out a function.
It seems that MatchingParen does not get set for parens surrounding a
macro-function parameter list. So for now, a loop is needed. I was able to
clean it up, though, and I've inlined the whole thing in the lambda.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist updated this revision to Diff 9.
enyquist marked 2 inline comments as done.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7518,8 +7518,104 @@
verifyFormat("a or_eq 8;", Spaces);
}
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AlignConsecutiveAssignments = true;
+ Style.AlignConsecutiveDeclarations = true;
+ Style.AlignConsecutiveMacros = false;
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Style);
+
+ Style.AlignConsecutiveMacros = false;
+ Style.ColumnLimit = 20;
+
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveAssignments = false;
verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -7702,6 +7798,7 @@
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveDeclarations = false;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -8659,6 +8756,7 @@
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
CHECK_PARSE_BOOL(AlignOperands);
CHECK_PARSE_BOOL(AlignTrailingComments);
+ CHECK_PARSE_BOOL(AlignConsecutiveMacros);
CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -165,6 +165,9 @@
/// \c EscapedNewlineColumn for the first tokens or token parts in a line.
void calculateLineBreakInformation();
+ /// \brief Align cons
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist updated this revision to Diff 93341.
enyquist added a comment.
Apologies-- forgot to update a comment
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7518,8 +7518,104 @@
verifyFormat("a or_eq 8;", Spaces);
}
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AlignConsecutiveAssignments = true;
+ Style.AlignConsecutiveDeclarations = true;
+ Style.AlignConsecutiveMacros = false;
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Style);
+
+ Style.AlignConsecutiveMacros = false;
+ Style.ColumnLimit = 20;
+
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveAssignments = false;
verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -7702,6 +7798,7 @@
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveDeclarations = false;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -8659,6 +8756,7 @@
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
CHECK_PARSE_BOOL(AlignOperands);
CHECK_PARSE_BOOL(AlignTrailingComments);
+ CHECK_PARSE_BOOL(AlignConsecutiveMacros);
CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -165,6 +165,9 @@
/// \c EscapedNewlineColumn for the first tokens or token parts in a line.
void calculateLineBreakInformation();
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist updated this revision to Diff 95680.
enyquist added a comment.
Rebased on latest
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7519,8 +7519,104 @@
verifyFormat("a or_eq 8;", Spaces);
}
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AlignConsecutiveAssignments = true;
+ Style.AlignConsecutiveDeclarations = true;
+ Style.AlignConsecutiveMacros = false;
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Style);
+
+ Style.AlignConsecutiveMacros = false;
+ Style.ColumnLimit = 20;
+
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveAssignments = false;
verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -7703,6 +7799,7 @@
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveDeclarations = false;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -8667,6 +8764,7 @@
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
CHECK_PARSE_BOOL(AlignOperands);
CHECK_PARSE_BOOL(AlignTrailingComments);
+ CHECK_PARSE_BOOL(AlignConsecutiveMacros);
CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -165,6 +165,9 @@
/// \c EscapedNewlineColumn for the first tokens or token parts in a line.
void calculateLineBreakInformation();
+ /// \brief Align c
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist updated this revision to Diff 84478.
enyquist added a comment.
Forgot to set "AlignConsecutiveMacros = false" in getLLVMStyle()
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
ClangFormatStyleOptions.rst
Format/Format.cpp
Format/FormatTest.cpp
Format/FormatToken.h
Format/TokenAnnotator.cpp
Format/WhitespaceManager.cpp
Format/WhitespaceManager.h
clang/Format/Format.h
Index: Format/FormatTest.cpp
===
--- Format/FormatTest.cpp
+++ Format/FormatTest.cpp
@@ -8532,8 +8532,78 @@
verifyFormat("a or_eq 8;", Spaces);
}
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+ FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveAssignments = true;
+ Alignment.AlignConsecutiveDeclarations = true;
+ Alignment.AlignConsecutiveMacros = false;
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Alignment);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Alignment);
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ Alignment.AlignConsecutiveMacros = true;
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Alignment);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Alignment);
+
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Alignment);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveAssignments = false;
verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -8674,7 +8744,10 @@
"int j = 2;",
Alignment);
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
" auto i = 0;\n"
" return 0;\n"
"};\n"
@@ -8710,6 +8783,7 @@
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveDeclarations = false;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -8869,7 +8943,10 @@
Alignment);
Alignment.AlignConsecutiveAssignments = true;
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
" auto ii = 0;\n"
" float j = 0;\n"
" return 0;\n"
@@ -9594,6 +9671,7 @@
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
CHECK_PARSE_BOOL(AlignOperands);
CHECK_PARSE_BOOL(AlignTrailingComments);
+ CHECK_PARSE_BOOL(AlignConsecutiveMacros);
CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: Format/WhitespaceManager.h
===
--- Format/WhitespaceManager.h
+++ Format/WhitespaceManager.h
@@ -107,7 +107,7 @@
unsigned NewlinesBefore, StringRef PreviousLinePostfix,
StringRef CurrentLinePrefix, tok::TokenKind Kind,
bool ContinuesPPDirective, bool IsStar
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added inline comments. Comment at: Format/FormatToken.h:148 + /// \brief Whether the token is the final token in the identifier of a PP + // macro. This will be either 1) the identifier token following the 'define' djasper wrote: > This adds a lot of code, runtime and memory even if you disable the alignment > of macros and I am slightly hesitant about that. Two reasons: > - It would be nicer if all of this logic was completely encapsulated in the > whitespace manager. > - We don't want to spent the computing resources if this is disabled. > > Can we remove all of what's in FormatToken.h and TokenAnnotator.cpp and > instead have a function in the whitespace manager, that identifies this > location in the code? My first attempt was changes in WhitespaceManager.cpp only, however I couldn't figure out a way to make this work without access to the full FormatToken in WhitespaceManager. Sounds like you're going to try it this week, so I'll watch for that change, and try to implement this style option in WhitespaceManager.cpp only, when I see it. Comment at: Format/WhitespaceManager.cpp:33 StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective, -bool IsStartOfDeclName, bool IsInsideToken) +bool IsStartOfDeclName, bool IsInsideToken, bool EndsPPIdentifier) : CreateReplacement(CreateReplacement), djasper wrote: > We have to stop adding more stuff here. I think it is better to store > reference to the actual FormatToken by now. I'll take a stab at that later > this week. Sounds good-- this style option should be do-able in WhitespaceManager only, if we have access to the full FormatToken. Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added a comment. Thanks :) I should get a chance to return to this next week. Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added a comment. Gaaah. I'm so sorry. I wrote that last comment months ago and never submitted. No wonder you guys weren't responding. Comment at: lib/Format/WhitespaceManager.cpp:470 // definitions. return C.Tok->is(TT_StartOfName) || C.Tok->is(TT_FunctionDeclarationName) || @djasper, this is my reasoning for the special case (whether or not it's a good reason, is open to discussion, but this is the reason anyway): AlignConsecutiveDeclarations only needs to look at one token, since the information required to determine a suitable token for this alignment is provided with the FormatToken. So, it doesn't matter if the current set of changes only contains one token (because, say, the previous portion was in a diff. scope and handled by a recursive invocation of AlignTokens). However, in order to determine an appropriate token in a PP macro, since these tokens are not annotated with the required information, the lambda function must attempt to crawl through the whole statement back to the '#define' keyword (meaning, all those tokens need to be processed in a single invocation of AlignTokens, without recurring). The best alternative I could see for this was to just annotate the FormatTokens with the needed info, which was in my initial implementation of this patch, however this adds unconditional overhead as you know. Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist marked 2 inline comments as done.
enyquist added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:413
+
+ while (Param && !Param->is(tok::l_paren)) {
+if (!Param->is(tok::identifier) && !Param->is(tok::comma))
djasper wrote:
> enyquist wrote:
> > djasper wrote:
> > > enyquist wrote:
> > > > djasper wrote:
> > > > > I think you should be able to use Current.MatchingParen here.
> > > > Hmm, I couldn't make this work... I just replaced this line with
> > > >
> > > > while (Param && Param != Current->MatchingParen)
> > > >
> > > > But it must not be doing what I think it's doing, since it made some
> > > > tests fail.
> > > > Mind you, my C-brain might be taking over here, please let me know if
> > > > I'm using MatchingParen incorrectly
> > > You shouldn't need a while loop. Just:
> > >
> > > if (!Current->MatchingParen || !Current->MatchingParen->Previous)
> > > return false;
> > > Param = Param->MatchingParen->Previous;
> > >
> > > And with that, I think you can simplify all these methods a bit:
> > >
> > > static bool endsPPIdentifier(const FormatToken *Current) {
> > > if (!Current->Next || Current->Next->SpacesRequiredBefore == 0)
> > > return false;
> > >
> > > // If Current is a "(", skip past the parameter list.
> > > if (Current->is(tok::r_paren)) {
> > > if (!Current->MatchingParen || !Current->MatchingParen->Previous)
> > > return false;
> > > Current = Current->MatchingParen->Previous;
> > > }
> > >
> > > const FormatToken *Keyword = Current->Previous;
> > > if (!Keyword || !Keyword->is(tok::pp_define))
> > > return false;
> > >
> > > return Current->is(tok::identifier);
> > > }
> > >
> > > Does that work? If so, I'd even suggest inlining this code directly into
> > > the lambda instead of pulling out a function.
> > It seems that MatchingParen does not get set for parens surrounding a
> > macro-function parameter list. So for now, a loop is needed. I was able to
> > clean it up, though, and I've inlined the whole thing in the lambda.
> Fixed that in r300661. We really should not add more ways to parse parens,
> especially not outside of unwrapped lines (e.g. in the whitespace manager).
> That can lead to very bad situations for error recovering when the code is
> incomplete.
Great, thanks for fixing. Your change works.
Comment at: lib/Format/WhitespaceManager.cpp:431
+
+ // Special case for AlignTokens: for all other alignment cases,
+ // the current sequence is ended when a comma or a scope change
djasper wrote:
> I am not yet sure I understand this. How is this different from:
>
> $ clang-format test.cc -style="{AlignConsecutiveDeclarations: true}"
> map m;
> map m;
> int a;
I'm not sure exactly what you mean. Do you mean, why do I need this special
case to ignore scope changes and commas? This was the only way I could get it
to work, AlignTokens was bailing out as soon as a paren or a comma inside
parens was seen.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist updated this revision to Diff 96333.
enyquist marked an inline comment as done.
enyquist added a comment.
Addressed comments
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7520,8 +7520,104 @@
verifyFormat("a or_eq 8;", Spaces);
}
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AlignConsecutiveAssignments = true;
+ Style.AlignConsecutiveDeclarations = true;
+ Style.AlignConsecutiveMacros = false;
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a 3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Style);
+
+ verifyFormat("#define a3\n"
+ "#define 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define (x, y) (x - y)",
+ Style);
+
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Style);
+
+ Style.AlignConsecutiveMacros = false;
+ Style.ColumnLimit = 20;
+
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+
+ Style.AlignConsecutiveMacros = true;
+ verifyFormat("#define a \\\n"
+ " \"aa\"\n"
+ "#define D \\\n"
+ " \"aa\" \\\n"
+ " \"ccdde\"\n"
+ "#define B \\\n"
+ " \"Q\" \\\n"
+ " \"F\" \\\n"
+ " \"\"\n",
+ Style);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveAssignments = false;
verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -7704,6 +7800,7 @@
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveDeclarations = false;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -8668,6 +8765,7 @@
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
CHECK_PARSE_BOOL(AlignOperands);
CHECK_PARSE_BOOL(AlignTrailingComments);
+ CHECK_PARSE_BOOL(AlignConsecutiveMacros);
CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -165,6 +165,9 @@
/// \c EscapedNewlineColumn for the first tokens or token parts in a line.
void calculateLin
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
enyquist added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:431
+
+ // Special case for AlignTokens: for all other alignment cases,
+ // the current sequence is ended when a comma or a scope change
djasper wrote:
> enyquist wrote:
> > djasper wrote:
> > > I am not yet sure I understand this. How is this different from:
> > >
> > > $ clang-format test.cc -style="{AlignConsecutiveDeclarations: true}"
> > > map m;
> > > map m;
> > > int a;
> > I'm not sure exactly what you mean. Do you mean, why do I need this special
> > case to ignore scope changes and commas? This was the only way I could get
> > it to work, AlignTokens was bailing out as soon as a paren or a comma
> > inside parens was seen.
> Yes, that's what I mean. The example I am writing above works correctly and
> structurally, the two should be the same, right? Maybe this is easier now
> that you can skip over the parameter list because of the correct
> MatchingParen?
Hmm... when I just set IgnoreScopeAndCommas to False, most of the tests fail
(all the blocks with parens in them) seem to fail. Sample:
```
/home/enyquist/clang-llvm/llvm/tools/clang/unittests/Format/FormatTest.cpp:74:
Failure
Expected: Code.str()
Which is: "#define a3\n#define 4\n#define ccc (5)"
To be equal to: format(test::messUp(Code), Style)
Which is: "#define a 3\n#define 4\n#define ccc (5)"
With diff:
@@ -1,3 +1,3 @@
-#define a3
+#define a 3
#define 4
-#define ccc (5)
+#define ccc (5)
```
Not sure exactly what's going on here, I'll look into it. Perhaps I don't need
to worry about commas.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
