[PATCH] D64384: [WIP] Index-while-building

[Jan Korous via Phabricator via llvm-commits]

jkorous added a comment.

I'm now taking a look at suggestions from https://reviews.llvm.org/D66125 regarding `IndexingFrontendAction` in libclang.

> IndexingFrontendAction is a lot more complex -- not only it needs a complex factory that creates the ASTConsumer, but it also overrides getTranslationUnitKind, which changes the behavior of Sema and Preprocessor in significant ways. So it does not seem like IndexFrontendAction can become an ASTConsumer.
> 
> However, IndexFrontendAction does not seem like a standalone ASTConsumer anyway. Its primary reason to exist is the logic in IndexingConsumer::shouldSkipFunctionBody. That logic seems interesting for general-purpose indexing, not just for libclang, so it would make sense to fold it into IndexASTConsumer.
> 
> To summarize:
> 
> Fold shouldSkipBody from IndexingConsumer in libclang into IndexASTConsumer in libIndex.
>  Make libIndex expose an ASTConsumer factory function, that encapsulates the logic currently in IndexingFrontendAction::CreateASTConsumer. Clients who need to combine indexing with some other FrontendAction can use that factory to create a consumer that will index whatever comes out of Sema.
>  Make a FrontendAction that implements the getTranslationUnitKind function to implement the corresponding optimization. If this FrontendAction is only needed in libclang, it can live there, otherwise, it can be in libIndex.
>  What do you think?

IIUC thegoal was to move as much of interesting logic to libIndex as possible and hopefully implement the rest of the `IndexingFrontendAction` as an `ASTConsumer` instead of `FrontendAction`.

Now, it seems to me:

- `IndexingFrontendAction::getTranslationUnitKind()` method just returns different result based on configuration, there's no logic - we just pass the configuration via `DataConsumer`. I am not quite sure this is interesting enough to create a self-standing FrontendAction.
- We could factor out an ASTConsumer factory from `IndexingFrontendAction::CreateASTConsumer` but there are also preprocessor callbacks created, `DataConsumer` set up with `CompilerInstance` and `CXIndexDataConsumer::importedPCH` called. This means we'd probably end up with having a FrontendAction with `CXIndexDataConsumer` in libclang anyway as this doesn't seem like a code that would quite fit into the hypothetical `ASTConsumer` factory.

Overall - it doesn't seem to me we could make `IndexingFrontendAction` significantly simpler or could reuse much of its code elsewhere so I suggest we leave it as it is.

WDYT?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64384/new/

https://reviews.llvm.org/D64384