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

Daniel Jasper via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 16 07:07:54 PST 2017

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;


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.



More information about the cfe-commits mailing list