https://github.com/HighCommander4 created https://github.com/llvm/llvm-project/pull/178132
This allows users of IndexAction who create the action but do not call it (e.g. the call will happen through ToolExecutor) to do some work when the action is complete, without assuming that a particular one of the individual callbacks (e.g. include graph) will be called last. >From 048b405f111fc8faae4d703455bd86363453677c Mon Sep 17 00:00:00 2001 From: Nathan Ridge <[email protected]> Date: Tue, 27 Jan 2026 02:54:55 -0500 Subject: [PATCH] [clangd] Unify IndexAction callbacks into one This allows users of IndexAction who create the action but do not call it (e.g. the call will happen through ToolExecutor) to do some work when the action is complete, without assuming that a particular one of the individual callbacks (e.g. include graph) will be called last. --- clang-tools-extra/clangd/index/Background.cpp | 5 +- .../clangd/index/IndexAction.cpp | 48 ++++++----------- clang-tools-extra/clangd/index/IndexAction.h | 10 ++-- .../clangd/indexer/IndexerMain.cpp | 54 +++++++++---------- .../clangd/unittests/IndexActionTests.cpp | 5 +- 5 files changed, 49 insertions(+), 73 deletions(-) diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp index 496d1455def4b..17a8097394492 100644 --- a/clang-tools-extra/clangd/index/Background.cpp +++ b/clang-tools-extra/clangd/index/Background.cpp @@ -309,10 +309,7 @@ llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) { IndexFileIn Index; auto Action = createStaticIndexingAction( - IndexOpts, [&](SymbolSlab S) { Index.Symbols = std::move(S); }, - [&](RefSlab R) { Index.Refs = std::move(R); }, - [&](RelationSlab R) { Index.Relations = std::move(R); }, - [&](IncludeGraph IG) { Index.Sources = std::move(IG); }); + IndexOpts, [&](IndexFileIn Result) { Index = std::move(Result); }); // We're going to run clang here, and it could potentially crash. // We could use CrashRecoveryContext to try to make indexing crashes nonfatal, diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp index 09943400f6d86..489c61f1ff424 100644 --- a/clang-tools-extra/clangd/index/IndexAction.cpp +++ b/clang-tools-extra/clangd/index/IndexAction.cpp @@ -11,6 +11,7 @@ #include "Headers.h" #include "clang-include-cleaner/Record.h" #include "index/Relation.h" +#include "index/Serialization.h" #include "index/SymbolCollector.h" #include "index/SymbolOrigin.h" #include "clang/AST/ASTConsumer.h" @@ -130,13 +131,8 @@ class IndexAction : public ASTFrontendAction { IndexAction(std::shared_ptr<SymbolCollector> C, std::unique_ptr<include_cleaner::PragmaIncludes> PI, const index::IndexingOptions &Opts, - std::function<void(SymbolSlab)> SymbolsCallback, - std::function<void(RefSlab)> RefsCallback, - std::function<void(RelationSlab)> RelationsCallback, - std::function<void(IncludeGraph)> IncludeGraphCallback) - : SymbolsCallback(SymbolsCallback), RefsCallback(RefsCallback), - RelationsCallback(RelationsCallback), - IncludeGraphCallback(IncludeGraphCallback), Collector(C), + std::function<void(IndexFileIn)> IndexContentsCallback) + : IndexContentsCallback(IndexContentsCallback), Collector(C), PI(std::move(PI)), Opts(Opts) { this->Opts.ShouldTraverseDecl = [this](const Decl *D) { // Many operations performed during indexing is linear in terms of depth @@ -161,9 +157,8 @@ class IndexAction : public ASTFrontendAction { std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override { PI->record(CI.getPreprocessor()); - if (IncludeGraphCallback != nullptr) - CI.getPreprocessor().addPPCallbacks( - std::make_unique<IncludeGraphCollector>(CI.getSourceManager(), IG)); + CI.getPreprocessor().addPPCallbacks( + std::make_unique<IncludeGraphCollector>(CI.getSourceManager(), IG)); return index::createIndexingASTConsumer(Collector, Opts, CI.getPreprocessorPtr()); @@ -185,26 +180,21 @@ class IndexAction : public ASTFrontendAction { } void EndSourceFileAction() override { - SymbolsCallback(Collector->takeSymbols()); - if (RefsCallback != nullptr) - RefsCallback(Collector->takeRefs()); - if (RelationsCallback != nullptr) - RelationsCallback(Collector->takeRelations()); - if (IncludeGraphCallback != nullptr) { + IndexFileIn Result; + Result.Symbols = Collector->takeSymbols(); + Result.Refs = Collector->takeRefs(); + Result.Relations = Collector->takeRelations(); #ifndef NDEBUG // This checks if all nodes are initialized. for (const auto &Node : IG) assert(Node.getKeyData() == Node.getValue().URI.data()); #endif - IncludeGraphCallback(std::move(IG)); - } + Result.Sources = std::move(IG); + IndexContentsCallback(std::move(Result)); } private: - std::function<void(SymbolSlab)> SymbolsCallback; - std::function<void(RefSlab)> RefsCallback; - std::function<void(RelationSlab)> RelationsCallback; - std::function<void(IncludeGraph)> IncludeGraphCallback; + std::function<void(IndexFileIn)> IndexContentsCallback; std::shared_ptr<SymbolCollector> Collector; std::unique_ptr<include_cleaner::PragmaIncludes> PI; index::IndexingOptions Opts; @@ -215,10 +205,7 @@ class IndexAction : public ASTFrontendAction { std::unique_ptr<FrontendAction> createStaticIndexingAction( SymbolCollector::Options Opts, - std::function<void(SymbolSlab)> SymbolsCallback, - std::function<void(RefSlab)> RefsCallback, - std::function<void(RelationSlab)> RelationsCallback, - std::function<void(IncludeGraph)> IncludeGraphCallback) { + std::function<void(IndexFileIn)> IndexContentsCallback) { index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; @@ -231,16 +218,13 @@ std::unique_ptr<FrontendAction> createStaticIndexingAction( if (Opts.Origin == SymbolOrigin::Unknown) Opts.Origin = SymbolOrigin::Static; Opts.StoreAllDocumentation = false; - if (RefsCallback != nullptr) { - Opts.RefFilter = RefKind::All; - Opts.RefsInHeaders = true; - } + Opts.RefFilter = RefKind::All; + Opts.RefsInHeaders = true; auto PragmaIncludes = std::make_unique<include_cleaner::PragmaIncludes>(); Opts.PragmaIncludes = PragmaIncludes.get(); return std::make_unique<IndexAction>(std::make_shared<SymbolCollector>(Opts), std::move(PragmaIncludes), IndexOpts, - SymbolsCallback, RefsCallback, - RelationsCallback, IncludeGraphCallback); + IndexContentsCallback); } } // namespace clangd diff --git a/clang-tools-extra/clangd/index/IndexAction.h b/clang-tools-extra/clangd/index/IndexAction.h index ecd1f63fb00f2..e6bf0b5a49d48 100644 --- a/clang-tools-extra/clangd/index/IndexAction.h +++ b/clang-tools-extra/clangd/index/IndexAction.h @@ -8,15 +8,16 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEXACTION_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEXACTION_H -#include "Headers.h" #include "index/SymbolCollector.h" #include "clang/Frontend/FrontendAction.h" namespace clang { namespace clangd { +struct IndexFileIn; + // Creates an action that indexes translation units and delivers the results -// for SymbolsCallback (each slab corresponds to one TU). +// for IndexContentsCallback (each call corresponds to one TU). // // Only a subset of SymbolCollector::Options are respected: // - include paths are always collected, and canonicalized appropriately @@ -25,10 +26,7 @@ namespace clangd { // - the symbol origin is set to Static if not specified by caller std::unique_ptr<FrontendAction> createStaticIndexingAction( SymbolCollector::Options Opts, - std::function<void(SymbolSlab)> SymbolsCallback, - std::function<void(RefSlab)> RefsCallback, - std::function<void(RelationSlab)> RelationsCallback, - std::function<void(IncludeGraph)> IncludeGraphCallback); + std::function<void(IndexFileIn)> IndexContentsCallback); } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/indexer/IndexerMain.cpp b/clang-tools-extra/clangd/indexer/IndexerMain.cpp index bc5d1a7408991..94db860f0b9b5 100644 --- a/clang-tools-extra/clangd/indexer/IndexerMain.cpp +++ b/clang-tools-extra/clangd/indexer/IndexerMain.cpp @@ -65,33 +65,33 @@ class IndexActionFactory : public tooling::FrontendActionFactory { std::lock_guard<std::mutex> Lock(FilesMu); return Files.insert(*AbsPath).second; // Skip already processed files. }; - return createStaticIndexingAction( - Opts, - [&](SymbolSlab S) { - // Merge as we go. - std::lock_guard<std::mutex> Lock(SymbolsMu); - for (const auto &Sym : S) { - if (const auto *Existing = Symbols.find(Sym.ID)) - Symbols.insert(mergeSymbol(*Existing, Sym)); - else - Symbols.insert(Sym); - } - }, - [&](RefSlab S) { - std::lock_guard<std::mutex> Lock(RefsMu); - for (const auto &Sym : S) { - // Deduplication happens during insertion. - for (const auto &Ref : Sym.second) - Refs.insert(Sym.first, Ref); - } - }, - [&](RelationSlab S) { - std::lock_guard<std::mutex> Lock(RelsMu); - for (const auto &R : S) { - Relations.insert(R); - } - }, - /*IncludeGraphCallback=*/nullptr); + return createStaticIndexingAction(Opts, [&](IndexFileIn Result) { + { + // Merge as we go. + std::lock_guard<std::mutex> Lock(SymbolsMu); + for (const auto &Sym : *Result.Symbols) { + if (const auto *Existing = Symbols.find(Sym.ID)) + Symbols.insert(mergeSymbol(*Existing, Sym)); + else + Symbols.insert(Sym); + } + } + { + std::lock_guard<std::mutex> Lock(RefsMu); + for (const auto &Sym : *Result.Refs) { + // Deduplication happens during insertion. + for (const auto &Ref : Sym.second) + Refs.insert(Sym.first, Ref); + } + } + { + std::lock_guard<std::mutex> Lock(RelsMu); + for (const auto &R : *Result.Relations) { + Relations.insert(R); + } + } + // FIXME: Handle Result.Sources? + }); } bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation, diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp index 2a9b8c9a1d338..025027fdcee5d 100644 --- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -86,10 +86,7 @@ class IndexActionTest : public ::testing::Test { new FileManager(FileSystemOptions(), InMemoryFileSystem)); auto Action = createStaticIndexingAction( - Opts, [&](SymbolSlab S) { IndexFile.Symbols = std::move(S); }, - [&](RefSlab R) { IndexFile.Refs = std::move(R); }, - [&](RelationSlab R) { IndexFile.Relations = std::move(R); }, - [&](IncludeGraph IG) { IndexFile.Sources = std::move(IG); }); + Opts, [&](IndexFileIn Result) { IndexFile = std::move(Result); }); std::vector<std::string> Args = {"index_action", "-fsyntax-only", "-xc++", "-std=c++11", _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
