[clangd] Heuristically resolve dependent call through smart pointer type

[nridge]

Summary

Fixes https://github.com/clangd/clangd/issues/227

[nridge]

clang-tools-extra/clangd/FindTarget.cpp

116

I don't intend for this function to be in the final patch.

Rather, I'm wondering: am I missing some obvious general way to recover the ASTContext from a Type (or a Stmt)?

If not, I think we'll need to modify targetDecl(), allTargetDecls(), findExplicitReferences() etc. to have their callers pass in an ASTContext?

[nridge]

Shopping around to other reviewers while Sam's OOO.

[sammccall]

This is great! Thank you!

clang-tools-extra/clangd/FindTarget.cpp

116

Haha, I see what you mean. There isn't a general way, but I think your hack is better than changing the public API, and we can improve it a little.

By the time you actually do the lookup, you have a CXXRecordDecl you're going to look into, and decls have a pointer to the ASTContext. So instead of a DeclarationName, you could pass a function_ref<const DeclarationName&(ASTContext&)> or such into getMembersReferencedViaDependentName, and have that function lazily call the factory to get the DeclarationName to use. Still hacky, but it's an internal function, and we don't have the weird parallel codepath...

clang-tools-extra/clangd/unittests/XRefsTests.cpp

503

you can remove the FIXME

[nridge]

Address review comments

clang-tools-extra/clangd/FindTarget.cpp

 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #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"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include <utility>
 
if (!RD->hasDefinition())
     return {};
   RD = RD->getDefinition();
   return RD->lookupDependentName(Name, [=](const NamedDecl *D) {
     return IsNonstaticMember ? D->isCXXInstanceMember()
                              : !D->isCXXInstanceMember();
   });
 }
 
+ASTContext *hackyFindASTContext(const Type *T) {
+  auto *TST = T->getAs<TemplateSpecializationType>();
+  if (!TST)
+    return nullptr;
+  const ClassTemplateDecl *TD = dyn_cast_or_null<ClassTemplateDecl>(
+      TST->getTemplateName().getAsTemplateDecl());
+  if (!TD)
+    return nullptr;
+  return &TD->getASTContext();
+}
+
+// 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.
+  ASTContext *Ctx = hackyFindASTContext(T);
+  if (!Ctx)
+    return nullptr;
+  auto ArrowOps = getMembersReferencedViaDependentName(
+      T, 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
 // may be more:
 //  - for ambiguous nodes like OverloadExpr
 //  - if we want to include e.g. both typedefs and the underlying type
 //
 // This is organized as a set of mutually recursive helpers for particular node
struct Visitor : public ConstStmtVisitor<Visitor> {
             // We don't know which designator was intended, we assume the outer.
             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();
+          BaseType = getPointeeType(BaseType);
         }
         for (const NamedDecl *D :
              getMembersReferencedViaDependentName(BaseType, E->getMember(),
                                                   /*IsNonstaticMember=*/true)) {
           Outer.add(D, Flags);
         }
       }
       void VisitDependentScopeDeclRefExpr(const DependentScopeDeclRefExpr *E) {

clang-tools-extra/clangd/unittests/XRefsTests.cpp

const char *Tests[] = {
         };
 
         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> 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"};