[PATCH] D48961: [Index] Add indexing support for MACROs.

[Eric Liu via Phabricator via cfe-commits]

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.


Repository:
  rC Clang

https://reviews.llvm.org/D48961