Index: clangd/index/SymbolCollector.h =================================================================== --- clangd/index/SymbolCollector.h +++ clangd/index/SymbolCollector.h @@ -111,7 +111,7 @@ private: const Symbol *addDeclaration(const NamedDecl &, SymbolID); - void addDefinition(const NamedDecl &, const Symbol &DeclSymbol); + void addDefinition(const NamedDecl &, SymbolID, const Symbol *); // All Symbols collected from the AST. SymbolSlab::Builder Symbols; Index: clangd/index/SymbolCollector.cpp =================================================================== --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -257,6 +257,13 @@ return false; } +Symbol createBasicSymbol(SymbolID ID, const std::string &QualifiedName) { + Symbol S; + S.ID = std::move(ID); + std::tie(S.Scope, S.Name) = splitQualifiedName(QualifiedName); + return S; +} + } // namespace SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {} @@ -353,9 +360,10 @@ // D may not be an interesting symbol, but it's cheaper to check at the end. auto &SM = ASTCtx->getSourceManager(); auto SpellingLoc = SM.getSpellingLoc(Loc); + auto FID = SM.getFileID(SpellingLoc); if (Opts.CountReferences && (Roles & static_cast(index::SymbolRole::Reference)) && - SM.getFileID(SpellingLoc) == SM.getMainFileID()) + FID == SM.getMainFileID()) ReferencedDecls.insert(ND); bool CollectRef = static_cast(Opts.RefFilter) & Roles; @@ -368,8 +376,10 @@ if (!shouldCollectSymbol(*ND, *ASTCtx, Opts)) return true; if (CollectRef && !isa(ND) && - (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID())) + (Opts.RefsInHeaders || FID == SM.getMainFileID()) && + shouldIndexFile(SM, FID, Opts, &FilesToIndexCache)) { DeclRefs[ND].emplace_back(SpellingLoc, Roles); + } // Don't continue indexing if this is a mere reference. if (IsOnlyRef) return true; @@ -390,7 +400,7 @@ BasicSymbol = addDeclaration(OriginalDecl, std::move(*ID)); if (Roles & static_cast(index::SymbolRole::Definition)) - addDefinition(OriginalDecl, *BasicSymbol); + addDefinition(OriginalDecl, *ID, BasicSymbol); return true; } @@ -438,8 +448,8 @@ S.Flags |= Symbol::IndexedForCodeCompletion; S.SymInfo = index::getSymbolInfoForMacro(*MI); std::string FileURI; - // FIXME: use the result to filter out symbols. - shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache); + if (!shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache)) + return true; if (auto DeclLoc = getTokenLocation(DefLoc, SM, Opts, PP->getLangOpts(), FileURI)) S.CanonicalDeclaration = *DeclLoc; @@ -516,8 +526,6 @@ if (auto ID = getSymbolID(It.first)) { for (const auto &LocAndRole : It.second) { auto FileID = SM.getFileID(LocAndRole.first); - // FIXME: use the result to filter out references. - shouldIndexFile(SM, FileID, Opts, &FilesToIndexCache); if (auto FileURI = GetURI(FileID)) { auto Range = getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts()); @@ -543,11 +551,13 @@ SymbolID ID) { auto &Ctx = ND.getASTContext(); auto &SM = Ctx.getSourceManager(); + auto Loc = findNameLoc(&ND); + if (!shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache)) + return nullptr; - Symbol S; - S.ID = std::move(ID); std::string QName = printQualifiedName(ND); - std::tie(S.Scope, S.Name) = splitQualifiedName(QName); + Symbol S = createBasicSymbol(std::move(ID), QName); + // FIXME: this returns foo:bar: for objective-C methods, we prefer only foo: // for consistency with CodeCompletionString and a clean name/signature split. @@ -557,9 +567,7 @@ S.Flags |= Symbol::ImplementationDetail; S.SymInfo = index::getSymbolInfo(&ND); std::string FileURI; - auto Loc = findNameLoc(&ND); - // FIXME: use the result to filter out symbols. - shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache); + if (auto DeclLoc = getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI)) S.CanonicalDeclaration = *DeclLoc; @@ -603,19 +611,23 @@ return Symbols.find(S.ID); } -void SymbolCollector::addDefinition(const NamedDecl &ND, - const Symbol &DeclSym) { - if (DeclSym.Definition) +void SymbolCollector::addDefinition(const NamedDecl &ND, SymbolID ID, + const Symbol *DeclSym) { + if (DeclSym && DeclSym->Definition) return; // If we saw some forward declaration, we end up copying the symbol. // This is not ideal, but avoids duplicating the "is this a definition" check // in clang::index. We should only see one definition. - Symbol S = DeclSym; std::string FileURI; auto Loc = findNameLoc(&ND); const auto &SM = ND.getASTContext().getSourceManager(); - // FIXME: use the result to filter out symbols. - shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache); + + if (!shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache)) + return; + + std::string QName = printQualifiedName(ND); + Symbol S = DeclSym ? *DeclSym : createBasicSymbol(std::move(ID), QName); + if (auto DefLoc = getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI)) S.Definition = *DefLoc; Index: unittests/clangd/SymbolCollectorTests.cpp =================================================================== --- unittests/clangd/SymbolCollectorTests.cpp +++ unittests/clangd/SymbolCollectorTests.cpp @@ -523,6 +523,59 @@ AllOf(QName("Z"), RefCount(0)), QName("y"))); } +TEST_F(SymbolCollectorTest, ShouldIndexFile) { + const std::string IgnoredHeader = R"( + class Foo {}; + void f(); + #define GLOBAL_Z(name) Z name; + )"; + + Annotations IndexedHeader(R"( + class $bar[[Bar]] {}; + void $b[[b]](); + )"); + Annotations Main(R"( + #include "ignored_header.h" + #include "indexed_header.h" + + void $f[[f]]() {} // only see definition + void $b[[b]]() { $f[[f]](); } + )"); + const std::string IgnoredHeaderPath = testPath("ignored_header.h"); + const std::string IndexedHeaderPath = testPath("indexed_header.h"); + const std::string IgnoredHeaderURI = + URI::createFile(IgnoredHeaderPath).toString(); + const std::string IndexedHeaderURI = + URI::createFile(IndexedHeaderPath).toString(); + + InMemoryFileSystem->addFile(IgnoredHeaderPath, 0, + MemoryBuffer::getMemBuffer(IgnoredHeader)); + InMemoryFileSystem->addFile(IndexedHeaderPath, 0, + MemoryBuffer::getMemBuffer(IndexedHeader.code())); + CollectorOpts.FileFilter = [](const SourceManager &SM, FileID FID) { + if (const auto *F = SM.getFileEntryForID(FID)) + return !F->getName().contains("ignored_header.h"); + return true; + }; + CollectorOpts.RefFilter = RefKind::All; + CollectorOpts.RefsInHeaders = true; + runSymbolCollector("", Main.code()); + EXPECT_THAT(Symbols, + UnorderedElementsAre( + AllOf(QName("f"), DefURI(TestFileURI), DeclURI("")), + AllOf(QName("Bar"), DeclRange(IndexedHeader.range("bar")), + DeclURI(IndexedHeaderURI)), + AllOf(QName("b"), DeclRange(IndexedHeader.range("b")), + DefRange(Main.range("b")), DeclURI(IndexedHeaderURI), + DefURI(TestFileURI)))); + EXPECT_THAT( + Refs, UnorderedElementsAre( + Pair(findSymbol(Symbols, "f").ID, HaveRanges(Main.ranges("f"))), + Pair(findSymbol(Symbols, "b").ID, _), + Pair(findSymbol(Symbols, "Bar").ID, + HaveRanges(IndexedHeader.ranges("bar"))))); +} + TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) { runSymbolCollector("class Foo {};", /*Main=*/""); EXPECT_THAT(Symbols, UnorderedElementsAre(