sammccall added a comment.

I worry this is a trap: the indexing infrastructure here is designed so you can 
run it as a frontendaction, on an ASTUnit, or by passing a set of top level 
decls.
However the macro functionality necessarily only works when running as a 
frontend action, so the same consumer would have different semantics when using 
these functions.
Moreover, clangd does call indexTopLevelDecls(), so this API might actually be 
awkward for us to use.

Alternatives would be:

- write lots of loud documentation and hope people read it
- punt on generalization and just do what we need in clangd with PPCallbacks 
directly
- offer a peer API here for consuming macros, and have the indexTopLevelDecls() 
etc only take the symbol consumer, createIndexingAction() would create both 
(and you could have a createIndexingPPCallbacks() that takes only the macro 
consumer).

WDYT?



================
Comment at: include/clang/Index/IndexDataConsumer.h:50
+                                    const MacroInfo &MI, SymbolRoleSet Roles,
+                                    SourceLocation Loc, bool Undefined = 
false);
 
----------------
I know this file isn't heavy on documentation, but I think `Undefined` needs 
some explanation/example where it might be true. And consider flipping to 
remove the negation.

Why a default parameter? Virtual + default is slightly confusing, and the 
#callers here should be few I think.


================
Comment at: include/clang/Index/IndexSymbol.h:138
 
+SymbolInfo getSymbolInfoForMacro();
+
----------------
this should take an arg - maybe macroinfo?
There are potentially subkinds here (function-like macros vs object-like ones) 
and it'll be harder to update callsites once they exist :-)


Repository:
  rC Clang

https://reviews.llvm.org/D48961



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to