[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks
MyDeveloperDay via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue May 5 03:11:58 PDT 2020
MyDeveloperDay accepted this revision.
MyDeveloperDay added a subscriber: VelocityRa.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.
I'm struggling with the simplicity of your solution ;-) (that's a compliment!)
I've pulled this patch and built it locally. It certainly had no detrimental effects on the sources I normally check clang-format against, and in some quick tests
#if FOO
int aaaaa = 12;
int bbbbbb = 12;
#else
int a = 12;
#endif
#if BAR
int aaavvvvvaaa = 12; // ABC
#else
int a = 12; // ABC
#endif
#if BAZ
int aaavvvvvaaa = 12;
#if FOO
int aaaaaavvvvvaaa = 12;
#endif
#else
int a = 12;
#endif
#if A
#else
int aaaaaaaaaaaa = 12; // ABC
#endif
#if B
#else
int a = 12; // ABC
#endif
#if B
#else
int verylongnamethatshouldalign = 12; // ABC
#endif
Looks to be transformed into:
#if FOO
int aaaaa = 12;
int bbbbbb = 12;
#else
int a = 12;
#endif
#if BAR
int aaavvvvvaaa = 12; // ABC
#else
int a = 12; // ABC
#endif
#if BAZ
int aaavvvvvaaa = 12;
#if FOO
int aaaaaavvvvvaaa = 12;
#endif
#else
int a = 12;
#endif
#if A
#else
int aaaaaaaaaaaa = 12; // ABC
#endif
#if B
#else
int a = 12; // ABC
#endif
#if B
#else
int verylongnamethatshouldalign = 12; // ABC
#endif
Which I think is your point, that the alignment shouldn't breach #endif and it definitely shouldn't inherit the alignment from something further down. (if I've understood correctly)
>From my perspective this LGTM, I'm impressed.
You may want to give some others a couple of days. (I'm guessing you don't have commit access correct?)
I'd like to add @VelocityRa & @enyquist as they wrote the AlignConsecutiveMacros work and probably have a greater insight than me into the problems around this area.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79388/new/
https://reviews.llvm.org/D79388
More information about the cfe-commits
mailing list