[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Erik Nyquist via Phabricator via cfe-commits]

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