diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp index 9f294d4ab92529..aa65008b51c004 100644 --- a/clang-tools-extra/clangd/index/IndexAction.cpp +++ b/clang-tools-extra/clangd/index/IndexAction.cpp @@ -132,11 +132,19 @@ class IndexAction : public ASTFrontendAction { std::function RefsCallback, std::function RelationsCallback, std::function IncludeGraphCallback) - : SymbolsCallback(SymbolsCallback), - RefsCallback(RefsCallback), RelationsCallback(RelationsCallback), + : SymbolsCallback(SymbolsCallback), RefsCallback(RefsCallback), + RelationsCallback(RelationsCallback), IncludeGraphCallback(IncludeGraphCallback), Collector(C), Includes(std::move(Includes)), Opts(Opts), - PragmaHandler(collectIWYUHeaderMaps(this->Includes.get())) {} + PragmaHandler(collectIWYUHeaderMaps(this->Includes.get())) { + this->Opts.ShouldTraverseDecl = [this](const Decl *D) { + auto &SM = D->getASTContext().getSourceManager(); + auto FID = SM.getFileID(SM.getExpansionLoc(D->getLocation())); + if (!FID.isValid()) + return true; + return Collector->shouldIndexFile(FID); + }; + } std::unique_ptr CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override { @@ -146,15 +154,8 @@ class IndexAction : public ASTFrontendAction { CI.getPreprocessor().addPPCallbacks( std::make_unique(CI.getSourceManager(), IG)); - 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); - }); + return index::createIndexingASTConsumer(Collector, Opts, + CI.getPreprocessorPtr()); } bool BeginInvocation(CompilerInstance &CI) override { diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp index 6441d019c7e186..31e1bc573290fb 100644 --- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -19,6 +19,7 @@ namespace { using ::testing::AllOf; using ::testing::ElementsAre; +using ::testing::EndsWith; using ::testing::Not; using ::testing::Pair; using ::testing::UnorderedElementsAre; @@ -75,8 +76,7 @@ class IndexActionTest : public ::testing::Test { new FileManager(FileSystemOptions(), InMemoryFileSystem)); auto Action = createStaticIndexingAction( - SymbolCollector::Options(), - [&](SymbolSlab S) { IndexFile.Symbols = std::move(S); }, + 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); }); @@ -99,11 +99,12 @@ class IndexActionTest : public ::testing::Test { void addFile(llvm::StringRef Path, llvm::StringRef Content) { InMemoryFileSystem->addFile(Path, 0, - llvm::MemoryBuffer::getMemBuffer(Content)); + llvm::MemoryBuffer::getMemBufferCopy(Content)); FilePaths.push_back(std::string(Path)); } protected: + SymbolCollector::Options Opts; std::vector FilePaths; llvm::IntrusiveRefCntPtr InMemoryFileSystem; }; @@ -250,6 +251,36 @@ TEST_F(IndexActionTest, NoWarnings) { EXPECT_THAT(*IndexFile.Symbols, ElementsAre(HasName("foo"), HasName("bar"))); } +TEST_F(IndexActionTest, SkipFiles) { + std::string MainFilePath = testPath("main.cpp"); + addFile(MainFilePath, R"cpp( + // clang-format off + #include "good.h" + #include "bad.h" + // clang-format on + )cpp"); + addFile(testPath("good.h"), R"cpp( + struct S { int s; }; + void f1() { S f; } + auto unskippable1() { return S(); } + )cpp"); + addFile(testPath("bad.h"), R"cpp( + struct T { S t; }; + void f2() { S f; } + auto unskippable2() { return S(); } + )cpp"); + Opts.FileFilter = [](const SourceManager &SM, FileID F) { + return !SM.getFileEntryForID(F)->getName().endswith("bad.h"); + }; + IndexFileIn IndexFile = runIndexingAction(MainFilePath, {"-std=c++14"}); + EXPECT_THAT(*IndexFile.Symbols, + UnorderedElementsAre(HasName("S"), HasName("s"), HasName("f1"), + HasName("unskippable1"))); + for (const auto &Pair : *IndexFile.Refs) + for (const auto &Ref : Pair.second) + EXPECT_THAT(Ref.Location.FileURI, EndsWith("good.h")); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang/include/clang/Index/IndexingAction.h b/clang/include/clang/Index/IndexingAction.h index 9ed2a018f1617d..4baa2d5e72603c 100644 --- a/clang/include/clang/Index/IndexingAction.h +++ b/clang/include/clang/Index/IndexingAction.h @@ -30,22 +30,21 @@ namespace serialization { } namespace index { - class IndexDataConsumer; +class IndexDataConsumer; /// Creates an ASTConsumer that indexes all symbols (macros and AST decls). +std::unique_ptr +createIndexingASTConsumer(std::shared_ptr DataConsumer, + const IndexingOptions &Opts, + std::shared_ptr PP); + std::unique_ptr createIndexingASTConsumer( std::shared_ptr DataConsumer, const IndexingOptions &Opts, std::shared_ptr PP, + // Prefer to set Opts.ShouldTraverseDecl and use the above overload. + // This version is only needed if used to *track* function body parsing. std::function ShouldSkipFunctionBody); -inline std::unique_ptr createIndexingASTConsumer( - std::shared_ptr DataConsumer, - const IndexingOptions &Opts, std::shared_ptr 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). std::unique_ptr createIndexingAction(std::shared_ptr DataConsumer, diff --git a/clang/include/clang/Index/IndexingOptions.h b/clang/include/clang/Index/IndexingOptions.h index bbfd6e4a72c621..2dd276998abf74 100644 --- a/clang/include/clang/Index/IndexingOptions.h +++ b/clang/include/clang/Index/IndexingOptions.h @@ -34,6 +34,12 @@ struct IndexingOptions { // Has no effect if IndexFunctionLocals are false. bool IndexParametersInDeclarations = false; bool IndexTemplateParameters = false; + + // If set, skip indexing inside some declarations for performance. + // This prevents traversal, so skipping a struct means its declaration an + // members won't be indexed, but references elsewhere to that struct will be. + // Currently this is only checked for top-level declarations. + std::function ShouldTraverseDecl; }; } // namespace index diff --git a/clang/lib/Index/IndexDecl.cpp b/clang/lib/Index/IndexDecl.cpp index 68160bc59eb6a9..2ba323e635753b 100644 --- a/clang/lib/Index/IndexDecl.cpp +++ b/clang/lib/Index/IndexDecl.cpp @@ -765,6 +765,9 @@ bool IndexingContext::indexTopLevelDecl(const Decl *D) { if (isa(D)) return true; // Wait for the objc container. + if (IndexOpts.ShouldTraverseDecl && !IndexOpts.ShouldTraverseDecl(D)) + return true; // skip + return indexDecl(D); } diff --git a/clang/lib/Index/IndexingAction.cpp b/clang/lib/Index/IndexingAction.cpp index 4f402135672c37..e698c07133a9c6 100644 --- a/clang/lib/Index/IndexingAction.cpp +++ b/clang/lib/Index/IndexingAction.cpp @@ -131,6 +131,21 @@ std::unique_ptr index::createIndexingASTConsumer( ShouldSkipFunctionBody); } +std::unique_ptr clang::index::createIndexingASTConsumer( + std::shared_ptr DataConsumer, + const IndexingOptions &Opts, std::shared_ptr PP) { + std::function ShouldSkipFunctionBody = [](const Decl *) { + return false; + }; + if (Opts.ShouldTraverseDecl) + ShouldSkipFunctionBody = + [ShouldTraverseDecl(Opts.ShouldTraverseDecl)](const Decl *D) { + return !ShouldTraverseDecl(D); + }; + return createIndexingASTConsumer(std::move(DataConsumer), Opts, std::move(PP), + std::move(ShouldSkipFunctionBody)); +} + std::unique_ptr index::createIndexingAction(std::shared_ptr DataConsumer, const IndexingOptions &Opts) {