[Index] Stopped wrapping FrontendActions in libIndex and its users

Exposed a new function, createIndexingASTConsumer, that creates an
ASTConsumer. ASTConsumers compose well.

Removed wrapping functionality from createIndexingAction.

llvm-svn: 370337

6 files changed, 81 insertions, 118 deletions

diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp
index be30ea639ad..b7ceb6f5d48 100644
--- a/clang-tools-extra/clangd/index/IndexAction.cpp
+++ b/clang-tools-extra/clangd/index/IndexAction.cpp
@@ -121,42 +121,32 @@ private:
   IncludeGraph &IG;
 };
 
-/// Returns an ASTConsumer that wraps \p Inner and additionally instructs the
-/// parser to skip bodies of functions in the files that should not be
-/// processed.
-static std::unique_ptr<ASTConsumer>
-skipProcessedFunctions(std::unique_ptr<ASTConsumer> Inner,
-                       std::function<bool(FileID)> ShouldIndexFile) {
-  class SkipProcessedFunctions : public ASTConsumer {
-  public:
-    SkipProcessedFunctions(std::function<bool(FileID)> FileFilter)
-        : ShouldIndexFile(std::move(FileFilter)), Context(nullptr) {
-      assert(this->ShouldIndexFile);
-    }
+/// An ASTConsumer that instructs the parser to skip bodies of functions in the
+/// files that should not be processed.
+class SkipProcessedFunctions : public ASTConsumer {
+public:
+  SkipProcessedFunctions(std::function<bool(FileID)> FileFilter)
+      : ShouldIndexFile(std::move(FileFilter)), Context(nullptr) {
+    assert(this->ShouldIndexFile);
+  }
 
-    void Initialize(ASTContext &Context) override { this->Context = &Context; }
-    bool shouldSkipFunctionBody(Decl *D) override {
-      assert(Context && "Initialize() was never called.");
-      auto &SM = Context->getSourceManager();
-      auto FID = SM.getFileID(SM.getExpansionLoc(D->getLocation()));
-      if (!FID.isValid())
-        return false;
-      return !ShouldIndexFile(FID);
-    }
+  void Initialize(ASTContext &Context) override { this->Context = &Context; }
+  bool shouldSkipFunctionBody(Decl *D) override {
+    assert(Context && "Initialize() was never called.");
+    auto &SM = Context->getSourceManager();
+    auto FID = SM.getFileID(SM.getExpansionLoc(D->getLocation()));
+    if (!FID.isValid())
+      return false;
+    return !ShouldIndexFile(FID);
+  }
 
-  private:
-    std::function<bool(FileID)> ShouldIndexFile;
-    const ASTContext *Context;
-  };
-  std::vector<std::unique_ptr<ASTConsumer>> Consumers;
-  Consumers.push_back(
-      std::make_unique<SkipProcessedFunctions>(ShouldIndexFile));
-  Consumers.push_back(std::move(Inner));
-  return std::make_unique<MultiplexConsumer>(std::move(Consumers));
-}
+private:
+  std::function<bool(FileID)> ShouldIndexFile;
+  const ASTContext *Context;
+};
 
 // Wraps the index action and reports index data after each translation unit.
-class IndexAction : public WrapperFrontendAction {
+class IndexAction : public ASTFrontendAction {
 public:
   IndexAction(std::shared_ptr<SymbolCollector> C,
               std::unique_ptr<CanonicalIncludes> Includes,
@@ -165,11 +155,10 @@ public:
               std::function<void(RefSlab)> RefsCallback,
               std::function<void(RelationSlab)> RelationsCallback,
               std::function<void(IncludeGraph)> IncludeGraphCallback)
-      : WrapperFrontendAction(index::createIndexingAction(C, Opts, nullptr)),
-        SymbolsCallback(SymbolsCallback), RefsCallback(RefsCallback),
-        RelationsCallback(RelationsCallback),
+      : SymbolsCallback(SymbolsCallback),
+        RefsCallback(RefsCallback), RelationsCallback(RelationsCallback),
         IncludeGraphCallback(IncludeGraphCallback), Collector(C),
-        Includes(std::move(Includes)),
+        Includes(std::move(Includes)), Opts(Opts),
         PragmaHandler(collectIWYUHeaderMaps(this->Includes.get())) {}
 
   std::unique_ptr<ASTConsumer>
@@ -179,9 +168,13 @@ public:
     if (IncludeGraphCallback != nullptr)
       CI.getPreprocessor().addPPCallbacks(
           std::make_unique<IncludeGraphCollector>(CI.getSourceManager(), IG));
-    return skipProcessedFunctions(
-        WrapperFrontendAction::CreateASTConsumer(CI, InFile),
-        [this](FileID FID) { return Collector->shouldIndexFile(FID); });
+
+    std::vector<std::unique_ptr<ASTConsumer>> Consumers;
+    Consumers.push_back(std::make_unique<SkipProcessedFunctions>(
+        [this](FileID FID) { return Collector->shouldIndexFile(FID); }));
+    Consumers.push_back(index::createIndexingASTConsumer(
+        Collector, Opts, CI.getPreprocessorPtr()));
+    return std::make_unique<MultiplexConsumer>(std::move(Consumers));
   }
 
   bool BeginInvocation(CompilerInstance &CI) override {
@@ -195,13 +188,10 @@ public:
     // bodies. The ASTConsumer will take care of skipping only functions inside
     // the files that we have already processed.
     CI.getFrontendOpts().SkipFunctionBodies = true;
-
-    return WrapperFrontendAction::BeginInvocation(CI);
+    return true;
   }
 
   void EndSourceFileAction() override {
-    WrapperFrontendAction::EndSourceFileAction();
-
     SymbolsCallback(Collector->takeSymbols());
     if (RefsCallback != nullptr)
       RefsCallback(Collector->takeRefs());
@@ -224,6 +214,7 @@ private:
   std::function<void(IncludeGraph)> IncludeGraphCallback;
   std::shared_ptr<SymbolCollector> Collector;
   std::unique_ptr<CanonicalIncludes> Includes;
+  index::IndexingOptions Opts;
   std::unique_ptr<CommentHandler> PragmaHandler;
   IncludeGraph IG;
 };
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 16b8ae8d677..a9248e1cfc0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -201,30 +201,30 @@ public:
       : COpts(std::move(COpts)), PragmaHandler(PragmaHandler) {}
 
   clang::FrontendAction *create() override {
-    class WrappedIndexAction : public WrapperFrontendAction {
+    class IndexAction : public ASTFrontendAction {
     public:
-      WrappedIndexAction(std::shared_ptr<SymbolCollector> C,
-                         const index::IndexingOptions &Opts,
-                         CommentHandler *PragmaHandler)
-          : WrapperFrontendAction(
-                index::createIndexingAction(C, Opts, nullptr)),
+      IndexAction(std::shared_ptr<index::IndexDataConsumer> DataConsumer,
+                  const index::IndexingOptions &Opts,
+                  CommentHandler *PragmaHandler)
+          : DataConsumer(std::move(DataConsumer)), Opts(Opts),
             PragmaHandler(PragmaHandler) {}
 
       std::unique_ptr<ASTConsumer>
       CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override {
         if (PragmaHandler)
           CI.getPreprocessor().addCommentHandler(PragmaHandler);
-        return WrapperFrontendAction::CreateASTConsumer(CI, InFile);
+        return createIndexingASTConsumer(DataConsumer, Opts, CI.getPreprocessorPtr());
       }
 
       bool BeginInvocation(CompilerInstance &CI) override {
         // Make the compiler parse all comments.
         CI.getLangOpts().CommentOpts.ParseAllComments = true;
-        return WrapperFrontendAction::BeginInvocation(CI);
+        return true;
       }
 
     private:
-      index::IndexingOptions IndexOpts;
+      std::shared_ptr<index::IndexDataConsumer> DataConsumer;
+      index::IndexingOptions Opts;
       CommentHandler *PragmaHandler;
     };
     index::IndexingOptions IndexOpts;
@@ -232,8 +232,7 @@ public:
         index::IndexingOptions::SystemSymbolFilterKind::All;
     IndexOpts.IndexFunctionLocals = false;
     Collector = std::make_shared<SymbolCollector>(COpts);
-    return new WrappedIndexAction(Collector, std::move(IndexOpts),
-                                  PragmaHandler);
+    return new IndexAction(Collector, std::move(IndexOpts), PragmaHandler);
   }
 
   std::shared_ptr<SymbolCollector> Collector;
diff --git a/clang/include/clang/Index/IndexingAction.h b/clang/include/clang/Index/IndexingAction.h
index 9756f3c539e..ce2ff0e105e 100644
--- a/clang/include/clang/Index/IndexingAction.h
+++ b/clang/include/clang/Index/IndexingAction.h
@@ -17,6 +17,7 @@
 
 namespace clang {
   class ASTContext;
+  class ASTConsumer;
   class ASTReader;
   class ASTUnit;
   class Decl;
@@ -49,12 +50,16 @@ struct IndexingOptions {
   bool IndexTemplateParameters = false;
 };
 
+/// Creates an ASTConsumer that indexes all symbols (macros and AST decls).
+std::unique_ptr<ASTConsumer>
+createIndexingASTConsumer(std::shared_ptr<IndexDataConsumer> DataConsumer,
+                          const IndexingOptions &Opts,
+                          std::shared_ptr<Preprocessor> PP);
+
 /// Creates a frontend action that indexes all symbols (macros and AST decls).
-/// \param WrappedAction another frontend action to wrap over or null.
 std::unique_ptr<FrontendAction>
 createIndexingAction(std::shared_ptr<IndexDataConsumer> DataConsumer,
-                     IndexingOptions Opts,
-                     std::unique_ptr<FrontendAction> WrappedAction);
+                     const IndexingOptions &Opts);
 
 /// Recursively indexes all decls in the AST.
 void indexASTUnit(ASTUnit &Unit, IndexDataConsumer &DataConsumer,
diff --git a/clang/lib/Index/IndexingAction.cpp b/clang/lib/Index/IndexingAction.cpp
index 8605a2cfa14..e7079720140 100644
--- a/clang/lib/Index/IndexingAction.cpp
+++ b/clang/lib/Index/IndexingAction.cpp
@@ -94,72 +94,38 @@ protected:
   }
 };
 
-class IndexActionBase {
-protected:
+class IndexAction final : public ASTFrontendAction {
   std::shared_ptr<IndexDataConsumer> DataConsumer;
   IndexingOptions Opts;
 
-  IndexActionBase(std::shared_ptr<IndexDataConsumer> DataConsumer,
-                  IndexingOptions Opts)
-      : DataConsumer(std::move(DataConsumer)), Opts(Opts) {
-    assert(this->DataConsumer != nullptr);
-  }
-
-  std::unique_ptr<IndexASTConsumer>
-  createIndexASTConsumer(CompilerInstance &CI) {
-    return std::make_unique<IndexASTConsumer>(DataConsumer, Opts,
-                                              CI.getPreprocessorPtr());
-  }
-};
-
-class IndexAction final : public ASTFrontendAction, IndexActionBase {
 public:
   IndexAction(std::shared_ptr<IndexDataConsumer> DataConsumer,
-              IndexingOptions Opts)
-    : IndexActionBase(std::move(DataConsumer), Opts) {}
-
-protected:
-  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
-                                                 StringRef InFile) override {
-    return createIndexASTConsumer(CI);
+              const IndexingOptions &Opts)
+      : DataConsumer(std::move(DataConsumer)), Opts(Opts) {
+    assert(this->DataConsumer != nullptr);
   }
-};
-
-class WrappingIndexAction final : public WrapperFrontendAction,
-                                  IndexActionBase {
-public:
-  WrappingIndexAction(std::unique_ptr<FrontendAction> WrappedAction,
-                      std::shared_ptr<IndexDataConsumer> DataConsumer,
-                      IndexingOptions Opts)
-    : WrapperFrontendAction(std::move(WrappedAction)),
-      IndexActionBase(std::move(DataConsumer), Opts) {}
 
 protected:
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                  StringRef InFile) override {
-    auto OtherConsumer = WrapperFrontendAction::CreateASTConsumer(CI, InFile);
-    if (!OtherConsumer) {
-      return nullptr;
-    }
-
-    std::vector<std::unique_ptr<ASTConsumer>> Consumers;
-    Consumers.push_back(std::move(OtherConsumer));
-    Consumers.push_back(createIndexASTConsumer(CI));
-    return std::make_unique<MultiplexConsumer>(std::move(Consumers));
+    return std::make_unique<IndexASTConsumer>(DataConsumer, Opts,
+                                              CI.getPreprocessorPtr());
   }
 };
 
 } // anonymous namespace
 
+std::unique_ptr<ASTConsumer>
+index::createIndexingASTConsumer(std::shared_ptr<IndexDataConsumer> DataConsumer,
+                          const IndexingOptions &Opts,
+                          std::shared_ptr<Preprocessor> PP) {
+  return std::make_unique<IndexASTConsumer>(DataConsumer, Opts, PP);
+}
+
 std::unique_ptr<FrontendAction>
 index::createIndexingAction(std::shared_ptr<IndexDataConsumer> DataConsumer,
-                            IndexingOptions Opts,
-                            std::unique_ptr<FrontendAction> WrappedAction) {
+                            const IndexingOptions &Opts) {
   assert(DataConsumer != nullptr);
-
-  if (WrappedAction)
-    return std::make_unique<WrappingIndexAction>(std::move(WrappedAction),
-                                                 std::move(DataConsumer), Opts);
   return std::make_unique<IndexAction>(std::move(DataConsumer), Opts);
 }
 
diff --git a/clang/tools/c-index-test/core_main.cpp b/clang/tools/c-index-test/core_main.cpp
index 2738e125cb9..f1edce0092c 100644
--- a/clang/tools/c-index-test/core_main.cpp
+++ b/clang/tools/c-index-test/core_main.cpp
@@ -221,9 +221,8 @@ static bool printSourceSymbols(const char *Executable,
   auto DataConsumer = std::make_shared<PrintIndexDataConsumer>(OS);
   IndexingOptions IndexOpts;
   IndexOpts.IndexFunctionLocals = indexLocals;
-  std::unique_ptr<FrontendAction> IndexAction;
-  IndexAction = createIndexingAction(DataConsumer, IndexOpts,
-                                     /*WrappedAction=*/nullptr);
+  std::unique_ptr<FrontendAction> IndexAction =
+      createIndexingAction(DataConsumer, IndexOpts);
 
   auto PCHContainerOps = std::make_shared<PCHContainerOperations>();
   std::unique_ptr<ASTUnit> Unit(ASTUnit::LoadFromCompilerInvocationAction(
diff --git a/clang/tools/libclang/Indexing.cpp b/clang/tools/libclang/Indexing.cpp
index ce1e6b3828f..372fb214a5a 100644
--- a/clang/tools/libclang/Indexing.cpp
+++ b/clang/tools/libclang/Indexing.cpp
@@ -19,6 +19,7 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/MultiplexConsumer.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -367,14 +368,16 @@ public:
 
 class IndexingFrontendAction : public ASTFrontendAction {
   std::shared_ptr<CXIndexDataConsumer> DataConsumer;
+  IndexingOptions Opts;
 
   SharedParsedRegionsStorage *SKData;
   std::unique_ptr<ParsedSrcLocationsTracker> ParsedLocsTracker;
 
 public:
   IndexingFrontendAction(std::shared_ptr<CXIndexDataConsumer> dataConsumer,
+                         const IndexingOptions &Opts,
                          SharedParsedRegionsStorage *skData)
-      : DataConsumer(std::move(dataConsumer)), SKData(skData) {}
+      : DataConsumer(std::move(dataConsumer)), Opts(Opts), SKData(skData) {}
 
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                  StringRef InFile) override {
@@ -398,8 +401,12 @@ public:
           std::make_unique<ParsedSrcLocationsTracker>(*SKData, *PPRec, PP);
     }
 
-    return std::make_unique<IndexingConsumer>(*DataConsumer,
-                                              ParsedLocsTracker.get());
+    std::vector<std::unique_ptr<ASTConsumer>> Consumers;
+    Consumers.push_back(std::make_unique<IndexingConsumer>(
+        *DataConsumer, ParsedLocsTracker.get()));
+    Consumers.push_back(
+        createIndexingASTConsumer(DataConsumer, Opts, CI.getPreprocessorPtr()));
+    return std::make_unique<MultiplexConsumer>(std::move(Consumers));
   }
 
   TranslationUnitKind getTranslationUnitKind() override {
@@ -569,12 +576,9 @@ static CXErrorCode clang_indexSourceFile_Impl(
   auto DataConsumer =
     std::make_shared<CXIndexDataConsumer>(client_data, CB, index_options,
                                           CXTU->getTU());
-  auto InterAction = std::make_unique<IndexingFrontendAction>(DataConsumer,
-                         SkipBodies ? IdxSession->SkipBodyData.get() : nullptr);
-  std::unique_ptr<FrontendAction> IndexAction;
-  IndexAction = createIndexingAction(DataConsumer,
-                                getIndexingOptionsFromCXOptions(index_options),
-                                     std::move(InterAction));
+  auto IndexAction = std::make_unique<IndexingFrontendAction>(
+      DataConsumer, getIndexingOptionsFromCXOptions(index_options),
+      SkipBodies ? IdxSession->SkipBodyData.get() : nullptr);
 
   // Recover resources if we crash before exiting this method.
   llvm::CrashRecoveryContextCleanupRegistrar<FrontendAction>
@@ -995,4 +999,3 @@ CXSourceLocation clang_indexLoc_getCXSourceLocation(CXIdxLoc location) {
       *static_cast<CXIndexDataConsumer*>(location.ptr_data[0]);
   return cxloc::translateSourceLocation(DataConsumer.getASTContext(), Loc);
 }
-