hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.
Previously, xrefs has inconsistent behavior when the reference is inside
macro body:
- AST-based xrefs (for main file) uses the expansion location;
- our index uses the spelling location;
This patch makes our index use expansion location, which is consistent with
AST-based xrefs, and kythe as well.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D70480
Files:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -644,6 +644,24 @@
HaveRanges(Header.ranges()))));
}
+TEST_F(SymbolCollectorTest, RefsOnMacros) {
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.RefsInHeaders = true;
+ Annotations Header(R"(
+ #define TYPE(X) X
+ #define FOO Foo
+ class [[Foo]] {};
+ void test() {
+ TYPE([[Foo]]) foo;
+ [[FOO]] foo2;
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ runSymbolCollector(Header.code(), "");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+ HaveRanges(Header.ranges()))));
+}
+
TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
CollectorOpts.RefFilter = RefKind::All;
Annotations Header(R"(
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -275,10 +275,9 @@
// 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.
auto &SM = ASTCtx->getSourceManager();
- auto SpellingLoc = SM.getSpellingLoc(Loc);
if (Opts.CountReferences &&
(Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
- SM.getFileID(SpellingLoc) == SM.getMainFileID())
+ SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
ReferencedDecls.insert(ND);
auto ID = getSymbolID(ND);
@@ -312,8 +311,9 @@
return true;
// Do not store references to main-file symbols.
if (CollectRef && !IsMainFileOnly && !isa<NamespaceDecl>(ND) &&
- (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
- DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+ (Opts.RefsInHeaders ||
+ SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID()))
+ DeclRefs[ND].emplace_back(SM.getFileLoc(Loc), Roles);
// Don't continue indexing if this is a mere reference.
if (IsOnlyRef)
return true;
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -644,6 +644,24 @@
HaveRanges(Header.ranges()))));
}
+TEST_F(SymbolCollectorTest, RefsOnMacros) {
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.RefsInHeaders = true;
+ Annotations Header(R"(
+ #define TYPE(X) X
+ #define FOO Foo
+ class [[Foo]] {};
+ void test() {
+ TYPE([[Foo]]) foo;
+ [[FOO]] foo2;
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ runSymbolCollector(Header.code(), "");
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+ HaveRanges(Header.ranges()))));
+}
+
TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
CollectorOpts.RefFilter = RefKind::All;
Annotations Header(R"(
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -275,10 +275,9 @@
// 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.
auto &SM = ASTCtx->getSourceManager();
- auto SpellingLoc = SM.getSpellingLoc(Loc);
if (Opts.CountReferences &&
(Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
- SM.getFileID(SpellingLoc) == SM.getMainFileID())
+ SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
ReferencedDecls.insert(ND);
auto ID = getSymbolID(ND);
@@ -312,8 +311,9 @@
return true;
// Do not store references to main-file symbols.
if (CollectRef && !IsMainFileOnly && !isa<NamespaceDecl>(ND) &&
- (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
- DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+ (Opts.RefsInHeaders ||
+ SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID()))
+ DeclRefs[ND].emplace_back(SM.getFileLoc(Loc), Roles);
// Don't continue indexing if this is a mere reference.
if (IsOnlyRef)
return true;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
