[clang-tools-extra] 16f47cf - [clangd] Heuristically resolve dependent call through smart pointer type

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-01-07T12:52:03-05:00
New Revision: 16f47cf607c7193e888de4c1774c46367a5bedf4

URL: https://github.com/llvm/llvm-project/commit/16f47cf607c7193e888de4c1774c46367a5bedf4
DIFF: https://github.com/llvm/llvm-project/commit/16f47cf607c7193e888de4c1774c46367a5bedf4.diff

LOG: [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

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 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<T>::size().
-std::vector<const NamedDecl *>
-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<const NamedDecl *> getMembersReferencedViaDependentName(
+    const Type *T,
+    llvm::function_ref<DeclarationName(ASTContext &)> NameFactory,
+    bool IsNonstaticMember) {
   if (!T)
     return {};
   if (auto *ICNT = T->getAs<InjectedClassNameType>()) {
@@ -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<PointerType>()->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<X, ...>, and assume X is the pointee type.
+  auto *TST = T->getAs<TemplateSpecializationType>();
+  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 @@ struct TargetFinder {
       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();
+          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 <typename> struct S { void foo(); };
+        template <typename> struct S { void [[foo]]() {} };
         template <typename T> struct unique_ptr {
           T* operator->();
         };