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

[Daniel Jasper via Phabricator via cfe-commits]

djasper 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'
----------------
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?


================
Comment at: Format/TokenAnnotator.cpp:1652
+    return false;
+  if (!tok.is(tok::identifier) || !tok.TokenText.equals("define"))
+    return false;
----------------
Use:

  tok.is(tok::pp_define)


================
Comment at: Format/TokenAnnotator.cpp:1661
+static bool endsMacroIdentifier(const FormatToken &Current) {
+  const FormatToken *keyword = Current.Previous;
+
----------------
Upper-case "Keyword"


================
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),
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D28462