Author: sammccall Date: Mon Mar 12 07:49:09 2018 New Revision: 327275 URL: http://llvm.org/viewvc/llvm-project?rev=327275&view=rev Log: [clangd] Collect the number of files referencing a symbol in the static index.
Summary: This is an important ranking signal. It's off for the dynamic index for now. Correspondingly, tell the index infrastructure only to report declarations for the dynamic index. Reviewers: ioeric, hokein Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits Differential Revision: https://reviews.llvm.org/D44315 Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp clang-tools-extra/trunk/clangd/index/FileIndex.cpp clang-tools-extra/trunk/clangd/index/Index.h clang-tools-extra/trunk/clangd/index/Merge.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.h clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=327275&r1=327274&r2=327275&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp (original) +++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp Mon Mar 12 07:49:09 2018 @@ -99,6 +99,7 @@ public: auto CollectorOpts = SymbolCollector::Options(); CollectorOpts.FallbackDir = AssumedHeaderDir; CollectorOpts.CollectIncludePath = true; + CollectorOpts.CountReferences = true; auto Includes = llvm::make_unique<CanonicalIncludes>(); addSystemHeadersMapping(Includes.get()); CollectorOpts.Includes = Includes.get(); Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=327275&r1=327274&r2=327275&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original) +++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Mon Mar 12 07:49:09 2018 @@ -26,12 +26,14 @@ std::unique_ptr<SymbolSlab> indexAST(AST // AST at this point, but we also need preprocessor callbacks (e.g. // CommentHandler for IWYU pragma) to canonicalize includes. CollectorOpts.CollectIncludePath = false; + CollectorOpts.CountReferences = false; auto Collector = std::make_shared<SymbolCollector>(std::move(CollectorOpts)); Collector->setPreprocessor(std::move(PP)); index::IndexingOptions IndexOpts; + // We only need declarations, because we don't count references. IndexOpts.SystemSymbolFilter = - index::IndexingOptions::SystemSymbolFilterKind::All; + index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly; IndexOpts.IndexFunctionLocals = false; index::indexTopLevelDecls(Ctx, Decls, Collector, IndexOpts); Modified: clang-tools-extra/trunk/clangd/index/Index.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=327275&r1=327274&r2=327275&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Index.h (original) +++ clang-tools-extra/trunk/clangd/index/Index.h Mon Mar 12 07:49:09 2018 @@ -131,6 +131,9 @@ struct Symbol { // * For non-inline functions, the canonical declaration typically appears // in the ".h" file corresponding to the definition. SymbolLocation CanonicalDeclaration; + // The number of translation units that reference this symbol from their main + // file. This number is only meaningful if aggregated in an index. + unsigned References = 0; /// A brief description of the symbol that can be displayed in the completion /// candidate list. For example, "Foo(X x, Y y) const" is a labal for a Modified: clang-tools-extra/trunk/clangd/index/Merge.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Merge.cpp?rev=327275&r1=327274&r2=327275&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Merge.cpp (original) +++ clang-tools-extra/trunk/clangd/index/Merge.cpp Mon Mar 12 07:49:09 2018 @@ -73,6 +73,7 @@ mergeSymbol(const Symbol &L, const Symbo S.Definition = O.Definition; if (!S.CanonicalDeclaration) S.CanonicalDeclaration = O.CanonicalDeclaration; + S.References += O.References; if (S.CompletionLabel == "") S.CompletionLabel = O.CompletionLabel; if (S.CompletionFilterText == "") Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=327275&r1=327274&r2=327275&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Mar 12 07:49:09 2018 @@ -228,37 +228,58 @@ bool SymbolCollector::handleDeclOccurenc ArrayRef<index::SymbolRelation> Relations, FileID FID, unsigned Offset, index::IndexDataConsumer::ASTNodeInfo ASTNode) { assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set."); + assert(CompletionAllocator && CompletionTUInfo); + const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D); + if (!ND) + return true; + + // Mark D as referenced if this is a reference coming from the main file. + // D may not be an interesting symbol, but it's cheaper to check at the end. + if (Opts.CountReferences && + (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) && + ASTCtx->getSourceManager().getMainFileID() == FID) + ReferencedDecls.insert(ND); - // FIXME: collect all symbol references. + // Don't continue indexing if this is a mere reference. if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) || Roles & static_cast<unsigned>(index::SymbolRole::Definition))) return true; + if (shouldFilterDecl(ND, ASTCtx, Opts)) + return true; - assert(CompletionAllocator && CompletionTUInfo); + llvm::SmallString<128> USR; + if (index::generateUSRForDecl(ND, USR)) + return true; + SymbolID ID(USR); - if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) { - if (shouldFilterDecl(ND, ASTCtx, Opts)) - return true; - llvm::SmallString<128> USR; - if (index::generateUSRForDecl(ND, USR)) - return true; + const NamedDecl &OriginalDecl = *cast<NamedDecl>(ASTNode.OrigD); + const Symbol *BasicSymbol = Symbols.find(ID); + if (!BasicSymbol) // Regardless of role, ND is the canonical declaration. + BasicSymbol = addDeclaration(*ND, std::move(ID)); + else if (isPreferredDeclaration(OriginalDecl, Roles)) + // If OriginalDecl is preferred, replace the existing canonical + // declaration (e.g. a class forward declaration). There should be at most + // one duplicate as we expect to see only one preferred declaration per + // TU, because in practice they are definitions. + BasicSymbol = addDeclaration(OriginalDecl, std::move(ID)); - const NamedDecl &OriginalDecl = *cast<NamedDecl>(ASTNode.OrigD); - auto ID = SymbolID(USR); - const Symbol *BasicSymbol = Symbols.find(ID); - if (!BasicSymbol) // Regardless of role, ND is the canonical declaration. - BasicSymbol = addDeclaration(*ND, std::move(ID)); - else if (isPreferredDeclaration(OriginalDecl, Roles)) - // If OriginalDecl is preferred, replace the existing canonical - // declaration (e.g. a class forward declaration). There should be at most - // one duplicate as we expect to see only one preferred declaration per - // TU, because in practice they are definitions. - BasicSymbol = addDeclaration(OriginalDecl, std::move(ID)); + if (Roles & static_cast<unsigned>(index::SymbolRole::Definition)) + addDefinition(OriginalDecl, *BasicSymbol); + return true; +} - if (Roles & static_cast<unsigned>(index::SymbolRole::Definition)) - addDefinition(OriginalDecl, *BasicSymbol); +void SymbolCollector::finish() { + // At the end of the TU, add 1 to the refcount of the ReferencedDecls. + for (const auto *ND : ReferencedDecls) { + llvm::SmallString<128> USR; + if (!index::generateUSRForDecl(ND, USR)) + if (const auto *S = Symbols.find(SymbolID(USR))) { + Symbol Inc = *S; + ++Inc.References; + Symbols.insert(Inc); + } } - return true; + ReferencedDecls.clear(); } const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=327275&r1=327274&r2=327275&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Mon Mar 12 07:49:09 2018 @@ -45,6 +45,8 @@ public: /// If set, this is used to map symbol #include path to a potentially /// different #include path. const CanonicalIncludes *Includes = nullptr; + // Populate the Symbol.References field. + bool CountReferences = false; }; SymbolCollector(Options Opts); @@ -63,6 +65,8 @@ public: SymbolSlab takeSymbols() { return std::move(Symbols).build(); } + void finish() override; + private: const Symbol *addDeclaration(const NamedDecl &, SymbolID); void addDefinition(const NamedDecl &, const Symbol &DeclSymbol); @@ -74,6 +78,8 @@ private: std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator; std::unique_ptr<CodeCompletionTUInfo> CompletionTUInfo; Options Opts; + // Decls referenced from the current TU, flushed on finish(). + llvm::DenseSet<const NamedDecl *> ReferencedDecls; }; } // namespace clangd Modified: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp?rev=327275&r1=327274&r2=327275&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp (original) +++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp Mon Mar 12 07:49:09 2018 @@ -100,6 +100,7 @@ template <> struct MappingTraits<Symbol> IO.mapOptional("CanonicalDeclaration", Sym.CanonicalDeclaration, SymbolLocation()); IO.mapOptional("Definition", Sym.Definition, SymbolLocation()); + IO.mapOptional("References", Sym.References, 0u); IO.mapRequired("CompletionLabel", Sym.CompletionLabel); IO.mapRequired("CompletionFilterText", Sym.CompletionFilterText); IO.mapRequired("CompletionPlainInsertText", Sym.CompletionPlainInsertText); Modified: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp?rev=327275&r1=327274&r2=327275&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp Mon Mar 12 07:49:09 2018 @@ -225,6 +225,8 @@ TEST(MergeTest, Merge) { L.Name = R.Name = "Foo"; // same in both L.CanonicalDeclaration.FileURI = "file:///left.h"; // differs R.CanonicalDeclaration.FileURI = "file:///right.h"; + L.References = 1; + R.References = 2; L.CompletionPlainInsertText = "f00"; // present in left only R.CompletionSnippetInsertText = "f0{$1:0}"; // present in right only Symbol::Details DetL, DetR; @@ -238,6 +240,7 @@ TEST(MergeTest, Merge) { Symbol M = mergeSymbol(L, R, &Scratch); EXPECT_EQ(M.Name, "Foo"); EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:///left.h"); + EXPECT_EQ(M.References, 3u); EXPECT_EQ(M.CompletionPlainInsertText, "f00"); EXPECT_EQ(M.CompletionSnippetInsertText, "f0{$1:0}"); ASSERT_TRUE(M.Detail); Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=327275&r1=327274&r2=327275&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Mon Mar 12 07:49:09 2018 @@ -59,6 +59,7 @@ MATCHER_P(DefRange, Offsets, "") { return arg.Definition.StartOffset == Offsets.first && arg.Definition.EndOffset == Offsets.second; } +MATCHER_P(Refs, R, "") { return int(arg.References) == R; } namespace clang { namespace clangd { @@ -201,7 +202,7 @@ TEST_F(SymbolCollectorTest, CollectSymbo TEST_F(SymbolCollectorTest, Template) { Annotations Header(R"( // Template is indexed, specialization and instantiation is not. - template <class T> struct [[Tmpl]] {T x = 0}; + template <class T> struct [[Tmpl]] {T x = 0;}; template <> struct Tmpl<int> {}; extern template struct Tmpl<float>; template struct Tmpl<double>; @@ -242,6 +243,31 @@ TEST_F(SymbolCollectorTest, Locations) { AllOf(QName("Z"), DeclRange(Header.offsetRange("zdecl"))))); } +TEST_F(SymbolCollectorTest, References) { + const std::string Header = R"( + class W; + class X {}; + class Y; + class Z {}; // not used anywhere + Y* y = nullptr; // used in header doesn't count + )"; + const std::string Main = R"( + W* w = nullptr; + W* w2 = nullptr; // only one usage counts + X x(); + class V; + V* v = nullptr; // Used, but not eligible for indexing. + class Y{}; // definition doesn't count as a reference + )"; + CollectorOpts.CountReferences = true; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, + UnorderedElementsAre(AllOf(QName("W"), Refs(1)), + AllOf(QName("X"), Refs(1)), + AllOf(QName("Y"), Refs(0)), + AllOf(QName("Z"), Refs(0)), QName("y"))); +} + TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) { runSymbolCollector("class Foo {};", /*Main=*/""); EXPECT_THAT(Symbols, UnorderedElementsAre( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
