[clang-tools-extra] ecaa936 - [clangd] Heuristically resolve dependent method calls
Nathan Ridge via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 12 14:18:44 PST 2019
Author: Nathan Ridge
Date: 2019-12-12T17:18:00-05:00
New Revision: ecaa9363303e811a051ebb6199e35e43319a699c
URL: https://github.com/llvm/llvm-project/commit/ecaa9363303e811a051ebb6199e35e43319a699c
DIFF: https://github.com/llvm/llvm-project/commit/ecaa9363303e811a051ebb6199e35e43319a699c.diff
LOG: [clangd] Heuristically resolve dependent method calls
Summary:
The heuristic is to look in the definition of the primary template,
which is what you want in the vast majority of cases.
Fixes https://github.com/clangd/clangd/issues/141
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D71240
Added:
Modified:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp
index 69c298b6887c..a4c17dbefded 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -52,6 +52,40 @@ nodeToString(const ast_type_traits::DynTypedNode &N) {
return S;
}
+// Given a dependent type and a member name, heuristically resolve the
+// name to one or more declarations.
+// The current heuristic is simply to look up the name in the primary
+// template. This is a heuristic because the template could potentially
+// have specializations that declare
diff erent members.
+// Multiple declarations could be returned if the name is overloaded
+// (e.g. an overloaded method in the primary template).
+// This heuristic will give the desired answer in many cases, e.g.
+// for a call to vector<T>::size().
+std::vector<const NamedDecl *>
+getMembersReferencedViaDependentName(const Type *T, const DeclarationName &Name,
+ bool IsNonstaticMember) {
+ if (!T)
+ return {};
+ if (auto *ICNT = T->getAs<InjectedClassNameType>()) {
+ T = ICNT->getInjectedSpecializationType().getTypePtrOrNull();
+ }
+ auto *TST = T->getAs<TemplateSpecializationType>();
+ if (!TST)
+ return {};
+ const ClassTemplateDecl *TD = dyn_cast_or_null<ClassTemplateDecl>(
+ TST->getTemplateName().getAsTemplateDecl());
+ if (!TD)
+ return {};
+ CXXRecordDecl *RD = TD->getTemplatedDecl();
+ if (!RD->hasDefinition())
+ return {};
+ RD = RD->getDefinition();
+ return RD->lookupDependentName(Name, [=](const NamedDecl *D) {
+ return IsNonstaticMember ? D->isCXXInstanceMember()
+ : !D->isCXXInstanceMember();
+ });
+}
+
// TargetFinder locates the entities that an AST node refers to.
//
// Typically this is (possibly) one declaration and (possibly) one type, but
@@ -79,9 +113,9 @@ nodeToString(const ast_type_traits::DynTypedNode &N) {
// formally size() is unresolved, but the primary template is a good guess.
// This affects:
// - DependentTemplateSpecializationType,
-// - DependentScopeMemberExpr
-// - DependentScopeDeclRefExpr
// - DependentNameType
+// - UnresolvedUsingValueDecl
+// - UnresolvedUsingTypenameDecl
struct TargetFinder {
using RelSet = DeclRelationSet;
using Rel = DeclRelation;
@@ -212,6 +246,32 @@ struct TargetFinder {
break;
}
}
+ void
+ VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
+ const Type *BaseType = E->getBaseType().getTypePtrOrNull();
+ if (E->isArrow()) {
+ // FIXME: Handle smart pointer types by looking up operator->
+ // in the primary template.
+ if (!BaseType || !BaseType->isPointerType()) {
+ return;
+ }
+ BaseType = BaseType->getAs<PointerType>()
+ ->getPointeeType()
+ .getTypePtrOrNull();
+ }
+ for (const NamedDecl *D :
+ getMembersReferencedViaDependentName(BaseType, E->getMember(),
+ /*IsNonstaticMember=*/true)) {
+ Outer.add(D, Flags);
+ }
+ }
+ void VisitDependentScopeDeclRefExpr(const DependentScopeDeclRefExpr *E) {
+ for (const NamedDecl *D : getMembersReferencedViaDependentName(
+ E->getQualifier()->getAsType(), E->getDeclName(),
+ /*IsNonstaticMember=*/false)) {
+ Outer.add(D, Flags);
+ }
+ }
void VisitObjCIvarRefExpr(const ObjCIvarRefExpr *OIRE) {
Outer.add(OIRE->getDecl(), Flags);
}
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 058f205bb3a6..0352322790bd 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -465,6 +465,51 @@ TEST(LocateSymbol, All) {
template <typename T>
struct Fo^o<T*> {};
+ )cpp",
+
+ R"cpp(// Heuristic resolution of dependent method
+ template <typename T>
+ struct S {
+ void [[bar]]() {}
+ };
+
+ template <typename T>
+ void foo(S<T> arg) {
+ arg.ba^r();
+ }
+ )cpp",
+
+ R"cpp(// Heuristic resolution of dependent method via this->
+ template <typename T>
+ struct S {
+ void [[foo]]() {
+ this->fo^o();
+ }
+ };
+ )cpp",
+
+ R"cpp(// Heuristic resolution of dependent static method
+ template <typename T>
+ struct S {
+ static void [[bar]]() {}
+ };
+
+ template <typename T>
+ void foo() {
+ S<T>::ba^r();
+ }
+ )cpp",
+
+ R"cpp(// FIXME: Heuristic resolution of dependent method
+ // invoked via smart pointer
+ template <typename> struct S { void foo(); };
+ template <typename T> struct unique_ptr {
+ T* operator->();
+ };
+ template <typename T>
+ void test(unique_ptr<S<T>>& V) {
+ V->fo^o();
+ }
)cpp"};
for (const char *Test : Tests) {
Annotations T(Test);
@@ -525,6 +570,21 @@ TEST(LocateSymbol, Ambiguous) {
Foo abcde$10^("asdf");
Foo foox2 = Foo$11^("asdf");
}
+
+ template <typename T>
+ struct S {
+ void $NonstaticOverload1[[bar]](int);
+ void $NonstaticOverload2[[bar]](float);
+
+ static void $StaticOverload1[[baz]](int);
+ static void $StaticOverload2[[baz]](float);
+ };
+
+ template <typename T, typename U>
+ void dependent_call(S<T> s, U u) {
+ s.ba$12^r(u);
+ S<T>::ba$13^z(u);
+ }
)cpp");
auto AST = TestTU::withCode(T.code()).build();
// Ordered assertions are deliberate: we expect a predictable order.
@@ -544,6 +604,15 @@ TEST(LocateSymbol, Ambiguous) {
ElementsAre(Sym("Foo", T.range("ConstructorLoc"))));
EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
ElementsAre(Sym("Foo", T.range("ConstructorLoc"))));
+ // These assertions are unordered because the order comes from
+ // CXXRecordDecl::lookupDependentName() which doesn't appear to provide
+ // an order guarantee.
+ EXPECT_THAT(locateSymbolAt(AST, T.point("12")),
+ UnorderedElementsAre(Sym("bar", T.range("NonstaticOverload1")),
+ Sym("bar", T.range("NonstaticOverload2"))));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("13")),
+ UnorderedElementsAre(Sym("baz", T.range("StaticOverload1")),
+ Sym("baz", T.range("StaticOverload2"))));
}
TEST(LocateSymbol, TemplateTypedefs) {
More information about the cfe-commits
mailing list