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: D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users.
Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66879 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" @@ -296,54 +297,20 @@ class IndexingConsumer : public ASTConsumer { CXIndexDataConsumer &DataConsumer; - ParsedSrcLocationsTracker *ParsedLocsTracker; public: IndexingConsumer(CXIndexDataConsumer &dataConsumer, ParsedSrcLocationsTracker *parsedLocsTracker) - : DataConsumer(dataConsumer), ParsedLocsTracker(parsedLocsTracker) {} - - // ASTConsumer Implementation + : DataConsumer(dataConsumer) {} void Initialize(ASTContext &Context) override { DataConsumer.setASTContext(Context); DataConsumer.startedTranslationUnit(); } - void HandleTranslationUnit(ASTContext &Ctx) override { - if (ParsedLocsTracker) - ParsedLocsTracker->syncWithStorage(); - } - bool HandleTopLevelDecl(DeclGroupRef DG) override { return !DataConsumer.shouldAbort(); } - - bool shouldSkipFunctionBody(Decl *D) override { - if (!ParsedLocsTracker) { - // Always skip bodies. - return true; - } - - const SourceManager &SM = DataConsumer.getASTContext().getSourceManager(); - SourceLocation Loc = D->getLocation(); - if (Loc.isMacroID()) - return false; - if (SM.isInSystemHeader(Loc)) - return true; // always skip bodies from system headers. - - FileID FID; - unsigned Offset; - std::tie(FID, Offset) = SM.getDecomposedLoc(Loc); - // Don't skip bodies from main files; this may be revisited. - if (SM.getMainFileID() == FID) - return false; - const FileEntry *FE = SM.getFileEntryForID(FID); - if (!FE) - return false; - - return ParsedLocsTracker->hasAlredyBeenParsed(Loc, FID, FE); - } }; //===----------------------------------------------------------------------===// @@ -367,14 +334,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 +367,39 @@ 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(), + [this](const Decl *D) { return this->shouldSkipFunctionBody(D); })); + return std::make_unique<MultiplexConsumer>(std::move(Consumers)); + } + + bool shouldSkipFunctionBody(const Decl *D) { + if (!ParsedLocsTracker) { + // Always skip bodies. + return true; + } + + const SourceManager &SM = D->getASTContext().getSourceManager(); + SourceLocation Loc = D->getLocation(); + if (Loc.isMacroID()) + return false; + if (SM.isInSystemHeader(Loc)) + return true; // always skip bodies from system headers. + + FileID FID; + unsigned Offset; + std::tie(FID, Offset) = SM.getDecomposedLoc(Loc); + // Don't skip bodies from main files; this may be revisited. + if (SM.getMainFileID() == FID) + return false; + const FileEntry *FE = SM.getFileEntryForID(FID); + if (!FE) + return false; + + return ParsedLocsTracker->hasAlredyBeenParsed(Loc, FID, FE); } TranslationUnitKind getTranslationUnitKind() override { @@ -409,6 +409,11 @@ return TU_Prefix; } bool hasCodeCompletionSupport() const override { return false; } + + void EndSourceFileAction() override { + if (ParsedLocsTracker) + ParsedLocsTracker->syncWithStorage(); + } }; //===----------------------------------------------------------------------===// @@ -569,12 +574,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 +997,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,88 @@ } }; -class IndexActionBase { -protected: +class IndexASTConsumer final : public ASTConsumer { std::shared_ptr<IndexDataConsumer> DataConsumer; std::shared_ptr<IndexingContext> IndexCtx; + std::shared_ptr<Preprocessor> PP; + std::function<bool(const Decl *)> ShouldSkipFunctionBody; - 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, + std::function<bool(const Decl *)> ShouldSkipFunctionBody) + : DataConsumer(std::move(DataConsumer)), + IndexCtx(new IndexingContext(Opts, *this->DataConsumer)), + PP(std::move(PP)), + ShouldSkipFunctionBody(std::move(ShouldSkipFunctionBody)) { + 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) {} + void HandleInterestingDecl(DeclGroupRef DG) override { + // Ignore deserialized decls. + } -protected: - std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI, - StringRef InFile) override { - return createIndexASTConsumer(CI); + void HandleTopLevelDeclInObjCContainer(DeclGroupRef DG) override { + IndexCtx->indexDeclGroupRef(DG); } - bool BeginSourceFileAction(clang::CompilerInstance &CI) override { - CI.getPreprocessor().addPPCallbacks(createIndexPPCallbacks()); - return true; + void HandleTranslationUnit(ASTContext &Ctx) override { + DataConsumer->finish(); } - void EndSourceFileAction() override { - FrontendAction::EndSourceFileAction(); - finish(); + bool shouldSkipFunctionBody(Decl *D) override { + return ShouldSkipFunctionBody(D); } }; -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(), + /*ShouldSkipFunctionBody=*/[](const Decl *) { return false; }); } }; } // anonymous namespace +std::unique_ptr<ASTConsumer> index::createIndexingASTConsumer( + std::shared_ptr<IndexDataConsumer> DataConsumer, + const IndexingOptions &Opts, std::shared_ptr<Preprocessor> PP, + std::function<bool(const Decl *)> ShouldSkipFunctionBody) { + return std::make_unique<IndexASTConsumer>(DataConsumer, Opts, PP, + ShouldSkipFunctionBody); +} + 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 @@ -9,6 +9,7 @@ #ifndef LLVM_CLANG_INDEX_INDEXINGACTION_H #define LLVM_CLANG_INDEX_INDEXINGACTION_H +#include "clang/AST/ASTConsumer.h" #include "clang/Basic/LLVM.h" #include "clang/Lex/PPCallbacks.h" #include "clang/Lex/Preprocessor.h" @@ -17,6 +18,7 @@ namespace clang { class ASTContext; + class ASTConsumer; class ASTReader; class ASTUnit; class Decl; @@ -49,12 +51,24 @@ 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, + std::function<bool(const Decl *)> ShouldSkipFunctionBody); + +inline std::unique_ptr<ASTConsumer> createIndexingASTConsumer( + std::shared_ptr<IndexDataConsumer> DataConsumer, + const IndexingOptions &Opts, std::shared_ptr<Preprocessor> PP) { + return createIndexingASTConsumer( + std::move(DataConsumer), Opts, std::move(PP), + /*ShouldSkipFunctionBody=*/[](const Decl *) { return false; }); +} + /// 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,8 @@ 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); - } - - 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)); -} - // 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 +131,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 +144,16 @@ 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); }); + + return index::createIndexingASTConsumer( + Collector, Opts, CI.getPreprocessorPtr(), + /*ShouldSkipFunctionBody=*/[this](const Decl *D) { + auto &SM = D->getASTContext().getSourceManager(); + auto FID = SM.getFileID(SM.getExpansionLoc(D->getLocation())); + if (!FID.isValid()) + return false; + return !Collector->shouldIndexFile(FID); + }); } bool BeginInvocation(CompilerInstance &CI) override { @@ -195,13 +167,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 +193,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