nridge updated this revision to Diff 356428.
nridge marked an inline comment as done.
nridge added a comment.
Addressed review comments, except for the one about using ContainerDC, for the
reason explained
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105083/new/
https://reviews.llvm.org/D105083
Files:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/CallHierarchyTests.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
@@ -810,8 +810,7 @@
};
EXPECT_EQ(Container("ref1a"),
findSymbol(Symbols, "f2").ID); // function body (call)
- // FIXME: This is wrongly contained by fptr and not f2.
- EXPECT_NE(Container("ref1b"),
+ EXPECT_EQ(Container("ref1b"),
findSymbol(Symbols, "f2").ID); // function body (address-of)
EXPECT_EQ(Container("ref2"),
findSymbol(Symbols, "v1").ID); // variable initializer
Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -26,6 +26,22 @@
namespace clang {
namespace clangd {
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
+ const CallHierarchyItem &Item) {
+ return Stream << Item.name << "@" << Item.selectionRange;
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
+ const CallHierarchyIncomingCall &Call) {
+ Stream << "{ from: " << Call.from << ", ranges: [";
+ for (const auto &R : Call.fromRanges) {
+ Stream << R;
+ Stream << ", ";
+ }
+ return Stream << "] }";
+}
+
namespace {
using ::testing::AllOf;
@@ -252,6 +268,40 @@
CheckCallHierarchy(*AST, CalleeC.point(), testPath("callee.cc"));
}
+TEST(CallHierarchy, CallInLocalVarDecl) {
+ // Tests that local variable declarations are not treated as callers
+ // (they're not indexed, so they can't be represented as call hierarchy
+ // items); instead, the caller should be the containing function.
+ // However, namespace-scope variable declarations should be treated as
+ // callers because those are indexed and there is no enclosing entity
+ // that would be a useful caller.
+ Annotations Source(R"cpp(
+ int call^ee();
+ void caller1() {
+ $call1[[callee]]();
+ }
+ void caller2() {
+ int localVar = $call2[[callee]]();
+ }
+ int caller3 = $call3[[callee]]();
+ )cpp");
+ TestTU TU = TestTU::withCode(Source.code());
+ auto AST = TU.build();
+ auto Index = TU.index();
+
+ std::vector<CallHierarchyItem> Items =
+ prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
+ ASSERT_THAT(Items, ElementsAre(WithName("callee")));
+
+ auto Incoming = incomingCalls(Items[0], Index.get());
+ ASSERT_THAT(
+ Incoming,
+ ElementsAre(
+ AllOf(From(WithName("caller1")), FromRanges(Source.range("call1"))),
+ AllOf(From(WithName("caller2")), FromRanges(Source.range("call2"))),
+ AllOf(From(WithName("caller3")), FromRanges(Source.range("call3")))));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -152,6 +152,24 @@
return None;
}
+// Given a ref contained in enclosing decl `Enclosing`, return
+// the decl that should be used as that ref's Ref::Container. This is
+// usually `Enclosing` itself, but in cases where `Enclosing` is not
+// indexed, we walk further up because Ref::Container should always be
+// an indexed symbol.
+const Decl *getRefContainer(const Decl *Enclosing,
+ const SymbolCollector::Options &Opts) {
+ while (Enclosing) {
+ const NamedDecl *ND = dyn_cast_or_null<NamedDecl>(Enclosing);
+ if (ND && SymbolCollector::shouldCollectSymbol(*ND, ND->getASTContext(),
+ Opts, true)) {
+ return ND;
+ }
+ Enclosing = dyn_cast_or_null<Decl>(Enclosing->getDeclContext());
+ }
+ return nullptr;
+}
+
} // namespace
// Encapsulates decisions about how to record header paths in the index,
@@ -477,8 +495,8 @@
!isa<NamespaceDecl>(ND) &&
(Opts.RefsInHeaders ||
SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID()))
- DeclRefs[ND].push_back(
- SymbolRef{SM.getFileLoc(Loc), Roles, ASTNode.Parent});
+ DeclRefs[ND].push_back(SymbolRef{SM.getFileLoc(Loc), Roles,
+ getRefContainer(ASTNode.Parent, Opts)});
// 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
