[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).
>   WDYT?

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.

  rC Clang


More information about the cfe-commits mailing list