hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.
Previously, we only collect refs of the symbols which are declared in
the preamble and referenced in the main file, it works well when the
main file is .cpp file.
However, when the main file is .h file (when opening a .h file in the
editor), we don't collect refs of the symbol declared in this file, so we miss
these refs in our dynamic index.
A typical scenario:
1. Open Foo.h (which contains class Foo)
2. Open Foo.cpp, call find references for Foo
And we only get refs from Foo.cpp.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D63818
Files:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolCollector.h
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
@@ -626,6 +626,33 @@
EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _))));
}
+
+TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
+ CollectorOpts.RefFilter = RefKind::All;
+ Annotations Header(R"(
+ class $Foo[[Foo]] {};
+
+ void $Func[[Func]]() {
+ $Foo[[Foo]] fo;
+ }
+ )");
+ // The main file is normal .cpp file, we shouldn't collect any refs of
symbols
+ // which are not declared in the preamble.
+ TestFileName = testPath("foo.cpp");
+ runSymbolCollector("", Header.code());
+ EXPECT_THAT(Refs, UnorderedElementsAre());
+
+ // Run the .h file as main file, we should collect the refs.
+ TestFileName = testPath("foo.h");
+ runSymbolCollector("", Header.code(),
+ /*ExtraArgs=*/{"-xobjective-c++-header"});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Func")));
+ EXPECT_THAT(Refs, UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID,
+ HaveRanges(Header.ranges("Foo"))),
+ Pair(findSymbol(Symbols, "Func").ID,
+ HaveRanges(Header.ranges("Func")))));
+}
+
TEST_F(SymbolCollectorTest, RefsInHeaders) {
CollectorOpts.RefFilter = RefKind::All;
CollectorOpts.RefsInHeaders = true;
Index: clang-tools-extra/clangd/index/SymbolCollector.h
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -134,9 +134,10 @@
void setIncludeLocation(const Symbol &S, SourceLocation);
// Indexed macros, to be erased if they turned out to be include guards.
llvm::DenseSet<const IdentifierInfo *> IndexedMacros;
- // All refs collected from the AST.
- // Only symbols declared in preamble (from #include) and referenced from the
- // main file will be included.
+ // All refs collected from the AST. It includes:
+ // 1) symbols declared in the preamble and referenced from the main file (
+ // which is not a header), or
+ // 2) symbols declared and referenced from the main file (which is a
header)
RefSlab::Builder Refs;
// All relations collected from the AST.
RelationSlab::Builder Relations;
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -314,10 +314,12 @@
if (IsOnlyRef && !CollectRef)
return true;
- // ND is the canonical (i.e. first) declaration. If it's in the main file,
- // then no public declaration was visible, so assume it's main-file only.
+ // ND is the canonical (i.e. first) declaration. If it's in the main file
+ // (which is not a header), then no public declaration was visible, so assume
+ // it's main-file only.
bool IsMainFileOnly =
- SM.isWrittenInMainFile(SM.getExpansionLoc(ND->getBeginLoc()));
+ SM.isWrittenInMainFile(SM.getExpansionLoc(ND->getBeginLoc())) &&
+ !ASTCtx->getLangOpts().IsHeaderFile;
// In C, printf is a redecl of an implicit builtin! So check OrigD instead.
if (ASTNode.OrigD->isImplicit() ||
!shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly))
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -626,6 +626,33 @@
EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _))));
}
+
+TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
+ CollectorOpts.RefFilter = RefKind::All;
+ Annotations Header(R"(
+ class $Foo[[Foo]] {};
+
+ void $Func[[Func]]() {
+ $Foo[[Foo]] fo;
+ }
+ )");
+ // The main file is normal .cpp file, we shouldn't collect any refs of symbols
+ // which are not declared in the preamble.
+ TestFileName = testPath("foo.cpp");
+ runSymbolCollector("", Header.code());
+ EXPECT_THAT(Refs, UnorderedElementsAre());
+
+ // Run the .h file as main file, we should collect the refs.
+ TestFileName = testPath("foo.h");
+ runSymbolCollector("", Header.code(),
+ /*ExtraArgs=*/{"-xobjective-c++-header"});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Func")));
+ EXPECT_THAT(Refs, UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID,
+ HaveRanges(Header.ranges("Foo"))),
+ Pair(findSymbol(Symbols, "Func").ID,
+ HaveRanges(Header.ranges("Func")))));
+}
+
TEST_F(SymbolCollectorTest, RefsInHeaders) {
CollectorOpts.RefFilter = RefKind::All;
CollectorOpts.RefsInHeaders = true;
Index: clang-tools-extra/clangd/index/SymbolCollector.h
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -134,9 +134,10 @@
void setIncludeLocation(const Symbol &S, SourceLocation);
// Indexed macros, to be erased if they turned out to be include guards.
llvm::DenseSet<const IdentifierInfo *> IndexedMacros;
- // All refs collected from the AST.
- // Only symbols declared in preamble (from #include) and referenced from the
- // main file will be included.
+ // All refs collected from the AST. It includes:
+ // 1) symbols declared in the preamble and referenced from the main file (
+ // which is not a header), or
+ // 2) symbols declared and referenced from the main file (which is a header)
RefSlab::Builder Refs;
// All relations collected from the AST.
RelationSlab::Builder Relations;
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -314,10 +314,12 @@
if (IsOnlyRef && !CollectRef)
return true;
- // ND is the canonical (i.e. first) declaration. If it's in the main file,
- // then no public declaration was visible, so assume it's main-file only.
+ // ND is the canonical (i.e. first) declaration. If it's in the main file
+ // (which is not a header), then no public declaration was visible, so assume
+ // it's main-file only.
bool IsMainFileOnly =
- SM.isWrittenInMainFile(SM.getExpansionLoc(ND->getBeginLoc()));
+ SM.isWrittenInMainFile(SM.getExpansionLoc(ND->getBeginLoc())) &&
+ !ASTCtx->getLangOpts().IsHeaderFile;
// In C, printf is a redecl of an implicit builtin! So check OrigD instead.
if (ASTNode.OrigD->isImplicit() ||
!shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
