nridge updated this revision to Diff 283134.
nridge added a comment.
Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83536/new/
https://reviews.llvm.org/D83536
Files:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/BackgroundIndexTests.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
@@ -624,15 +624,13 @@
EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _))));
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID,
HaveRanges(Main.ranges("macro")))));
- // Symbols *only* in the main file:
- // - (a, b) externally visible and should have refs.
- // - (c, FUNC) externally invisible and had no refs collected.
- auto MainSymbols =
- TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
- EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "a").ID, _)));
- EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "b").ID, _)));
- EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _))));
- EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "FUNC").ID, _))));
+ // Symbols *only* in the main file (a, b, c) should have refs collected
+ // as well.
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "a").ID, _)));
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "b").ID, _)));
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "c").ID, _)));
+ // However, references to main-file macros are not collected.
+ EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "FUNC").ID, _))));
}
TEST_F(SymbolCollectorTest, MacroRefInHeader) {
@@ -708,12 +706,12 @@
llvm::StringRef Main;
llvm::StringRef TargetSymbolName;
} TestCases[] = {
- {
- R"cpp(
+ {
+ R"cpp(
struct Foo;
#define MACRO Foo
)cpp",
- R"cpp(
+ R"cpp(
struct $spelled[[Foo]] {
$spelled[[Foo]]();
~$spelled[[Foo]]();
@@ -721,24 +719,24 @@
$spelled[[Foo]] Variable1;
$implicit[[MACRO]] Variable2;
)cpp",
- "Foo",
- },
- {
- R"cpp(
+ "Foo",
+ },
+ {
+ R"cpp(
class Foo {
public:
Foo() = default;
};
)cpp",
- R"cpp(
+ R"cpp(
void f() { Foo $implicit[[f]]; f = $spelled[[Foo]]();}
)cpp",
- "Foo::Foo" /// constructor.
- },
+ "Foo::Foo" /// constructor.
+ },
};
CollectorOpts.RefFilter = RefKind::All;
CollectorOpts.RefsInHeaders = false;
- for (const auto& T : TestCases) {
+ for (const auto &T : TestCases) {
Annotations Header(T.Header);
Annotations Main(T.Main);
// Reset the file system.
@@ -818,8 +816,9 @@
$Foo[[Foo]] fo;
}
)");
- // The main file is normal .cpp file, we should collect the refs
- // for externally visible symbols.
+ // We should collect refs to main-file symbols in all cases:
+
+ // 1. The main file is normal .cpp file.
TestFileName = testPath("foo.cpp");
runSymbolCollector("", Header.code());
EXPECT_THAT(Refs,
@@ -828,7 +827,7 @@
Pair(findSymbol(Symbols, "Func").ID,
HaveRanges(Header.ranges("Func")))));
- // Run the .h file as main file, we should collect the refs.
+ // 2. Run the .h file as main file.
TestFileName = testPath("foo.h");
runSymbolCollector("", Header.code(),
/*ExtraArgs=*/{"-xobjective-c++-header"});
@@ -839,8 +838,7 @@
Pair(findSymbol(Symbols, "Func").ID,
HaveRanges(Header.ranges("Func")))));
- // Run the .hh file as main file (without "-x c++-header"), we should collect
- // the refs as well.
+ // 3. Run the .hh file as main file (without "-x c++-header").
TestFileName = testPath("foo.hh");
runSymbolCollector("", Header.code());
EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Func")));
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -199,7 +199,7 @@
EXPECT_THAT(runFuzzyFind(Idx, ""),
UnorderedElementsAre(AllOf(Named("common"), NumReferences(1U)),
AllOf(Named("A_CC"), NumReferences(0U)),
- AllOf(Named("g"), NumReferences(0U)),
+ AllOf(Named("g"), NumReferences(1U)),
AllOf(Named("f_b"), Declared(),
Not(Defined()), NumReferences(0U))));
@@ -212,7 +212,7 @@
EXPECT_THAT(runFuzzyFind(Idx, ""),
UnorderedElementsAre(AllOf(Named("common"), NumReferences(5U)),
AllOf(Named("A_CC"), NumReferences(0U)),
- AllOf(Named("g"), NumReferences(0U)),
+ AllOf(Named("g"), NumReferences(1U)),
AllOf(Named("f_b"), Declared(), Defined(),
NumReferences(1U))));
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -309,13 +309,11 @@
if (IsOnlyRef && !CollectRef)
return true;
- // Do not store references to main-file symbols.
// Unlike other fields, e.g. Symbols (which use spelling locations), we use
// file locations for references (as it aligns the behavior of clangd's
// AST-based xref).
// FIXME: we should try to use the file locations for other fields.
- if (CollectRef && (!IsMainFileOnly || ND->isExternallyVisible()) &&
- !isa<NamespaceDecl>(ND) &&
+ if (CollectRef && !isa<NamespaceDecl>(ND) &&
(Opts.RefsInHeaders ||
SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID()))
DeclRefs[ND].emplace_back(SM.getFileLoc(Loc), Roles);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
