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

[Erik Nyquist via Phabricator via cfe-commits]

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