[PATCH] D33440: clang-format: better handle statement macros

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Out of curiosity, will this be able to fix the two situations that you get for python extension?
There, you usually have a PyObject_HEAD with out semicolon in a struct and than a PyObject_HEAD_INIT(..) in a braced init list. More info:
http://starship.python.net/crew/mwh/toext/node20.html



================
Comment at: lib/Format/FormatTokenLexer.cpp:642
               tok::pp_define) &&
-        std::find(ForEachMacros.begin(), ForEachMacros.end(),
-                  FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end()) {
-      FormatTok->Type = TT_ForEachMacro;
+        (it = std::find(Macros.begin(), Macros.end(),
+                        FormatTok->Tok.getIdentifierInfo())) != Macros.end()) {
----------------
Typz wrote:
> djasper wrote:
> > This does a binary search. Why aren't you implementing it with a hashtable?
> It was already done this way, so I did not change it to avoid any impact on performance.
> But I can change it if you prefer.
Thanks. This is much better than it was before.


================
Comment at: lib/Format/FormatTokenLexer.cpp:630
   if (Style.isCpp()) {
+    decltype(Macros)::iterator it;
     if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() &&
----------------
Just do:

  auto it = Macros.find(FormatTok->Tok.getIdentifierInfo());
  ..

I know that this means that we might do the hash look up when we wouldn't need to (when we are actually in a #define), but I think clarity here is more important than the tiny performance benefit.


================
Comment at: lib/Format/FormatTokenLexer.h:25
 #include "llvm/Support/Regex.h"
+#include <llvm/ADT/MapVector.h>
 
----------------
Use ".


================
Comment at: lib/Format/UnwrappedLineParser.cpp:1254
 
+      if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+        nextToken();
----------------
This contains the exact same code (I think). Can you pull it out into a function?


https://reviews.llvm.org/D33440