[PATCH] D48961: [Index] Add indexing support for MACROs.
Eric Liu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 6 01:22:40 PDT 2018
ioeric added a comment.
Thanks for the review!
In https://reviews.llvm.org/D48961#1152999, @sammccall wrote:
> 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).
As chatted offline, the fact that `indexTopLevelDecls` etc can't index macros is unfortunate, but it seems to be by the current design and probably due to how preprocessor and AST work. As you suggested, I added some comments to make this more explicit and also exposed a function that returns a PPCallbacks for indexing macros, which should be useful for clangd's dynamic index. Hope this would make these functions harder to misuse.
More information about the cfe-commits