[clang-tools-extra] r327275 - [clangd] Collect the number of files referencing a symbol in the static index.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 12 07:49:09 PDT 2018
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(
More information about the cfe-commits
mailing list