[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

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
+/// An ASTConsumer that instructs the parser to skip bodies of functions in the
-/// parser to skip bodies of functions in the files that should not be
+/// files that should not be processed.
-/// processed.
+class SkipProcessedFunctions : public ASTConsumer {
-static std::unique_ptr<ASTConsumer>
+public:
-skipProcessedFunctions(std::unique_ptr<ASTConsumer> Inner,
+  SkipProcessedFunctions(std::function<bool(FileID)> FileFilter)
-                       std::function<bool(FileID)> ShouldIndexFile) {
+      : ShouldIndexFile(std::move(FileFilter)), Context(nullptr) {
-  class SkipProcessedFunctions : public ASTConsumer {
+    assert(this->ShouldIndexFile);
-  public:
+  }
-    SkipProcessedFunctions(std::function<bool(FileID)> FileFilter)
-        : ShouldIndexFile(std::move(FileFilter)), Context(nullptr) {
-      assert(this->ShouldIndexFile);
-    }
 
-    void Initialize(ASTContext &Context) override { this->Context = &Context; }
+  void Initialize(ASTContext &Context) override { this->Context = &Context; }
-    bool shouldSkipFunctionBody(Decl *D) override {
+  bool shouldSkipFunctionBody(Decl *D) override {
-      assert(Context && "Initialize() was never called.");
+    assert(Context && "Initialize() was never called.");
-      auto &SM = Context->getSourceManager();
+    auto &SM = Context->getSourceManager();
-      auto FID = SM.getFileID(SM.getExpansionLoc(D->getLocation()));
+    auto FID = SM.getFileID(SM.getExpansionLoc(D->getLocation()));
-      if (!FID.isValid())
+    if (!FID.isValid())
-        return false;
+      return false;
-      return !ShouldIndexFile(FID);
+    return !ShouldIndexFile(FID);
-    }
+  }
 
-  private:
+private:
-    std::function<bool(FileID)> ShouldIndexFile;
+  std::function<bool(FileID)> ShouldIndexFile;
-    const ASTContext *Context;
+  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));
-}
 
 // 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),
-        SymbolsCallback(SymbolsCallback), RefsCallback(RefsCallback),
+        RefsCallback(RefsCallback), RelationsCallback(RelationsCallback),
-        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),
+    std::vector<std::unique_ptr<ASTConsumer>> Consumers;
-        [this](FileID FID) { return Collector->shouldIndexFile(FID); });
+    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 true;
-    return WrapperFrontendAction::BeginInvocation(CI);
   }
 
   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;
 };

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,
+      IndexAction(std::shared_ptr<index::IndexDataConsumer> DataConsumer,
-                         const index::IndexingOptions &Opts,
+                  const index::IndexingOptions &Opts,
-                         CommentHandler *PragmaHandler)
+                  CommentHandler *PragmaHandler)
-          : WrapperFrontendAction(
+          : DataConsumer(std::move(DataConsumer)), Opts(Opts),
-                index::createIndexingAction(C, Opts, nullptr)),
             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),
+    return new IndexAction(Collector, std::move(IndexOpts), PragmaHandler);
-                                  PragmaHandler);
   }
 
   std::shared_ptr<SymbolCollector> Collector;

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,
+                     const IndexingOptions &Opts);
-                     std::unique_ptr<FrontendAction> WrappedAction);
 
 /// Recursively indexes all decls in the AST.
 void indexASTUnit(ASTUnit &Unit, IndexDataConsumer &DataConsumer,

clang/lib/Index/IndexingAction.cpp

@@ -94,72 +94,38 @@ protected:
   }
 };
 
-class IndexActionBase {
+class IndexAction final : public ASTFrontendAction {
-protected:
   std::shared_ptr<IndexDataConsumer> DataConsumer;
   IndexingOptions Opts;
 
-  IndexActionBase(std::shared_ptr<IndexDataConsumer> DataConsumer,
+public:
-                  IndexingOptions Opts)
+  IndexAction(std::shared_ptr<IndexDataConsumer> DataConsumer,
+              const IndexingOptions &Opts)
       : DataConsumer(std::move(DataConsumer)), Opts(Opts) {
     assert(this->DataConsumer != nullptr);
   }
 
-  std::unique_ptr<IndexASTConsumer>
+protected:
-  createIndexASTConsumer(CompilerInstance &CI) {
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 StringRef InFile) override {
     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);
-  }
-};
-
-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));
-  }
-};
-
 } // 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,
+                            const IndexingOptions &Opts) {
-                            std::unique_ptr<FrontendAction> WrappedAction) {
   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);
 }
 

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;
+  std::unique_ptr<FrontendAction> IndexAction =
-  IndexAction = createIndexingAction(DataConsumer, IndexOpts,
+      createIndexingAction(DataConsumer, IndexOpts);
-                                     /*WrappedAction=*/nullptr);
 
   auto PCHContainerOps = std::make_shared<PCHContainerOperations>();
   std::unique_ptr<ASTUnit> Unit(ASTUnit::LoadFromCompilerInvocationAction(

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,
+    std::vector<std::unique_ptr<ASTConsumer>> Consumers;
-                                              ParsedLocsTracker.get());
+    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,
+  auto IndexAction = std::make_unique<IndexingFrontendAction>(
-                         SkipBodies ? IdxSession->SkipBodyData.get() : nullptr);
+      DataConsumer, getIndexingOptionsFromCXOptions(index_options),
-  std::unique_ptr<FrontendAction> IndexAction;
+      SkipBodies ? IdxSession->SkipBodyData.get() : nullptr);
-  IndexAction = createIndexingAction(DataConsumer,
-                                getIndexingOptionsFromCXOptions(index_options),
-                                     std::move(InterAction));
 
   // 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);
 }
-