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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
