From 16f47cf607c7193e888de4c1774c46367a5bedf4 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sun, 15 Dec 2019 20:42:25 -0500 Subject: [PATCH] [clangd] Heuristically resolve dependent call through smart pointer type Summary: Fixes https://github.com/clangd/clangd/issues/227 Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D71644 --- clang-tools-extra/clangd/FindTarget.cpp | 72 +++++++++++++++---- .../clangd/unittests/XRefsTests.cpp | 4 +- 2 files changed, 59 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp index 75ea78c44a4e..fb3001fca94c 100644 --- a/clang-tools-extra/clangd/FindTarget.cpp +++ b/clang-tools-extra/clangd/FindTarget.cpp @@ -27,6 +27,7 @@ #include "clang/AST/TypeLoc.h" #include "clang/AST/TypeLocVisitor.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/OperatorKinds.h" #include "clang/Basic/SourceLocation.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" @@ -61,9 +62,14 @@ nodeToString(const ast_type_traits::DynTypedNode &N) { // (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::size(). -std::vector -getMembersReferencedViaDependentName(const Type *T, const DeclarationName &Name, - bool IsNonstaticMember) { +// The name to look up is provided in the form of a factory that takes +// an ASTContext, because an ASTContext may be needed to obtain the +// name (e.g. if it's an operator name), but the caller may not have +// access to an ASTContext. +std::vector getMembersReferencedViaDependentName( + const Type *T, + llvm::function_ref NameFactory, + bool IsNonstaticMember) { if (!T) return {}; if (auto *ICNT = T->getAs()) { @@ -80,12 +86,54 @@ getMembersReferencedViaDependentName(const Type *T, const DeclarationName &Name, if (!RD->hasDefinition()) return {}; RD = RD->getDefinition(); + DeclarationName Name = NameFactory(RD->getASTContext()); return RD->lookupDependentName(Name, [=](const NamedDecl *D) { return IsNonstaticMember ? D->isCXXInstanceMember() : !D->isCXXInstanceMember(); }); } +// Given the type T of a dependent expression that appears of the LHS of a "->", +// heuristically find a corresponding pointee type in whose scope we could look +// up the name appearing on the RHS. +const Type *getPointeeType(const Type *T) { + if (!T) + return nullptr; + + if (T->isPointerType()) { + return T->getAs()->getPointeeType().getTypePtrOrNull(); + } + + // Try to handle smart pointer types. + + // Look up operator-> in the primary template. If we find one, it's probably a + // smart pointer type. + auto ArrowOps = getMembersReferencedViaDependentName( + T, + [](ASTContext &Ctx) { + return Ctx.DeclarationNames.getCXXOperatorName(OO_Arrow); + }, + /*IsNonStaticMember=*/true); + if (ArrowOps.empty()) + return nullptr; + + // Getting the return type of the found operator-> method decl isn't useful, + // because we discarded template arguments to perform lookup in the primary + // template scope, so the return type would just have the form U* where U is a + // template parameter type. + // Instead, just handle the common case where the smart pointer type has the + // form of SmartPtr, and assume X is the pointee type. + auto *TST = T->getAs(); + if (!TST) + return nullptr; + if (TST->getNumArgs() == 0) + return nullptr; + const TemplateArgument &FirstArg = TST->getArg(0); + if (FirstArg.getKind() != TemplateArgument::Type) + return nullptr; + return FirstArg.getAsType().getTypePtrOrNull(); +} + // TargetFinder locates the entities that an AST node refers to. // // Typically this is (possibly) one declaration and (possibly) one type, but @@ -251,24 +299,18 @@ public: 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() - ->getPointeeType() - .getTypePtrOrNull(); + BaseType = getPointeeType(BaseType); } - for (const NamedDecl *D : - getMembersReferencedViaDependentName(BaseType, E->getMember(), - /*IsNonstaticMember=*/true)) { + for (const NamedDecl *D : getMembersReferencedViaDependentName( + BaseType, [E](ASTContext &) { return E->getMember(); }, + /*IsNonstaticMember=*/true)) { Outer.add(D, Flags); } } void VisitDependentScopeDeclRefExpr(const DependentScopeDeclRefExpr *E) { for (const NamedDecl *D : getMembersReferencedViaDependentName( - E->getQualifier()->getAsType(), E->getDeclName(), + E->getQualifier()->getAsType(), + [E](ASTContext &) { return E->getDeclName(); }, /*IsNonstaticMember=*/false)) { Outer.add(D, Flags); } diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index a37b80a99c43..6d16fa513737 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -485,9 +485,9 @@ TEST(LocateSymbol, All) { } )cpp", - R"cpp(// FIXME: Heuristic resolution of dependent method + R"cpp(// Heuristic resolution of dependent method // invoked via smart pointer - template struct S { void foo(); }; + template struct S { void [[foo]]() {} }; template struct unique_ptr { T* operator->(); };