[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