gribozavr created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous.
Herald added a project: clang.
gribozavr added reviewers: ilya-biryukov, jkorous.
Herald added a subscriber: dexonsmith.
gribozavr added a parent revision: D66877: Moved the IndexDataConsumer::finish
call into the IndexASTConsumer from IndexAction.
gribozavr added a child revision: D66879: [Index] Added a
ShouldSkipFunctionBody callback to libIndex, and refactored clients to use it
instead of inventing their own solution.
Exposed a new function, createIndexingASTConsumer, that creates an
ASTConsumer. ASTConsumers compose well.
Removed wrapping functionality from createIndexingAction.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D66878
Files:
clang-tools-extra/clangd/index/IndexAction.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
clang/include/clang/Index/IndexingAction.h
clang/lib/Index/IndexingAction.cpp
clang/tools/c-index-test/core_main.cpp
clang/tools/libclang/Indexing.cpp
Index: clang/tools/libclang/Indexing.cpp
===================================================================
--- clang/tools/libclang/Indexing.cpp
+++ 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 @@
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 @@
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 @@
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 @@
*static_cast<CXIndexDataConsumer*>(location.ptr_data[0]);
return cxloc::translateSourceLocation(DataConsumer.getASTContext(), Loc);
}
-
Index: clang/tools/c-index-test/core_main.cpp
===================================================================
--- clang/tools/c-index-test/core_main.cpp
+++ clang/tools/c-index-test/core_main.cpp
@@ -221,9 +221,8 @@
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(
Index: clang/lib/Index/IndexingAction.cpp
===================================================================
--- clang/lib/Index/IndexingAction.cpp
+++ clang/lib/Index/IndexingAction.cpp
@@ -23,39 +23,7 @@
namespace {
-class IndexASTConsumer : public ASTConsumer {
- std::shared_ptr<Preprocessor> PP;
- std::shared_ptr<IndexingContext> IndexCtx;
-
-public:
- IndexASTConsumer(std::shared_ptr<Preprocessor> PP,
- std::shared_ptr<IndexingContext> IndexCtx)
- : PP(std::move(PP)), IndexCtx(std::move(IndexCtx)) {}
-
-protected:
- void Initialize(ASTContext &Context) override {
- IndexCtx->setASTContext(Context);
- IndexCtx->getDataConsumer().initialize(Context);
- IndexCtx->getDataConsumer().setPreprocessor(PP);
- }
-
- bool HandleTopLevelDecl(DeclGroupRef DG) override {
- return IndexCtx->indexDeclGroupRef(DG);
- }
-
- void HandleInterestingDecl(DeclGroupRef DG) override {
- // Ignore deserialized decls.
- }
-
- void HandleTopLevelDeclInObjCContainer(DeclGroupRef DG) override {
- IndexCtx->indexDeclGroupRef(DG);
- }
-
- void HandleTranslationUnit(ASTContext &Ctx) override {
- }
-};
-
-class IndexPPCallbacks : public PPCallbacks {
+class IndexPPCallbacks final : public PPCallbacks {
std::shared_ptr<IndexingContext> IndexCtx;
public:
@@ -85,103 +53,79 @@
}
};
-class IndexActionBase {
-protected:
+class IndexASTConsumer final : public ASTConsumer {
std::shared_ptr<IndexDataConsumer> DataConsumer;
std::shared_ptr<IndexingContext> IndexCtx;
+ std::shared_ptr<Preprocessor> PP;
- IndexActionBase(std::shared_ptr<IndexDataConsumer> dataConsumer,
- IndexingOptions Opts)
- : DataConsumer(std::move(dataConsumer)),
- IndexCtx(new IndexingContext(Opts, *DataConsumer)) {}
-
- std::unique_ptr<IndexASTConsumer>
- createIndexASTConsumer(CompilerInstance &CI) {
- return std::make_unique<IndexASTConsumer>(CI.getPreprocessorPtr(),
- IndexCtx);
+public:
+ IndexASTConsumer(std::shared_ptr<IndexDataConsumer> DataConsumer,
+ const IndexingOptions &Opts,
+ std::shared_ptr<Preprocessor> PP)
+ : DataConsumer(std::move(DataConsumer)),
+ IndexCtx(new IndexingContext(Opts, *this->DataConsumer)),
+ PP(std::move(PP)) {
+ assert(this->DataConsumer != nullptr);
+ assert(this->PP != nullptr);
}
- std::unique_ptr<PPCallbacks> createIndexPPCallbacks() {
- return std::make_unique<IndexPPCallbacks>(IndexCtx);
+protected:
+ void Initialize(ASTContext &Context) override {
+ IndexCtx->setASTContext(Context);
+ IndexCtx->getDataConsumer().initialize(Context);
+ IndexCtx->getDataConsumer().setPreprocessor(PP);
+ PP->addPPCallbacks(std::make_unique<IndexPPCallbacks>(IndexCtx));
}
- void finish() {
- DataConsumer->finish();
+ bool HandleTopLevelDecl(DeclGroupRef DG) override {
+ return IndexCtx->indexDeclGroupRef(DG);
}
-};
-
-class IndexAction : 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);
+ void HandleInterestingDecl(DeclGroupRef DG) override {
+ // Ignore deserialized decls.
}
- bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
- CI.getPreprocessor().addPPCallbacks(createIndexPPCallbacks());
- return true;
+ void HandleTopLevelDeclInObjCContainer(DeclGroupRef DG) override {
+ IndexCtx->indexDeclGroupRef(DG);
}
- void EndSourceFileAction() override {
- FrontendAction::EndSourceFileAction();
- finish();
+ void HandleTranslationUnit(ASTContext &Ctx) override {
+ DataConsumer->finish();
}
};
-class WrappingIndexAction : public WrapperFrontendAction, IndexActionBase {
- bool IndexActionFailed = false;
+class IndexAction final : public ASTFrontendAction {
+ std::shared_ptr<IndexDataConsumer> DataConsumer;
+ IndexingOptions Opts;
public:
- WrappingIndexAction(std::unique_ptr<FrontendAction> WrappedAction,
- std::shared_ptr<IndexDataConsumer> DataConsumer,
- IndexingOptions Opts)
- : WrapperFrontendAction(std::move(WrappedAction)),
- IndexActionBase(std::move(DataConsumer), Opts) {}
+ IndexAction(std::shared_ptr<IndexDataConsumer> DataConsumer,
+ const IndexingOptions &Opts)
+ : DataConsumer(std::move(DataConsumer)), Opts(Opts) {
+ assert(this->DataConsumer != nullptr);
+ }
protected:
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) override {
- auto OtherConsumer = WrapperFrontendAction::CreateASTConsumer(CI, InFile);
- if (!OtherConsumer) {
- IndexActionFailed = true;
- 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));
- }
-
- bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
- WrapperFrontendAction::BeginSourceFileAction(CI);
- CI.getPreprocessor().addPPCallbacks(createIndexPPCallbacks());
- return true;
- }
-
- void EndSourceFileAction() override {
- // Invoke wrapped action's method.
- WrapperFrontendAction::EndSourceFileAction();
- if (!IndexActionFailed)
- finish();
+ 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) {
- if (WrappedAction)
- return std::make_unique<WrappingIndexAction>(std::move(WrappedAction),
- std::move(DataConsumer),
- Opts);
+ const IndexingOptions &Opts) {
+ assert(DataConsumer != nullptr);
return std::make_unique<IndexAction>(std::move(DataConsumer), Opts);
}
Index: clang/include/clang/Index/IndexingAction.h
===================================================================
--- clang/include/clang/Index/IndexingAction.h
+++ 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 @@
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,
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -201,30 +201,30 @@
: 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 @@
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;
Index: clang-tools-extra/clangd/index/IndexAction.cpp
===================================================================
--- clang-tools-extra/clangd/index/IndexAction.cpp
+++ clang-tools-extra/clangd/index/IndexAction.cpp
@@ -121,42 +121,32 @@
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 @@
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 @@
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 @@
// 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 @@
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;
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
