hokein created this revision.
hokein added a reviewer: ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54300
Files:
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/SymbolCollectorTests.cpp
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(
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<unsigned>(index::SymbolRole::Reference)) &&
- SM.getFileID(SpellingLoc) == SM.getMainFileID())
+ FID == SM.getMainFileID())
ReferencedDecls.insert(ND);
bool CollectRef = static_cast<unsigned>(Opts.RefFilter) & Roles;
@@ -368,8 +376,10 @@
if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
return true;
if (CollectRef && !isa<NamespaceDecl>(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<unsigned>(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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
