[clangd] Target member of dependent base made visible via a using-decl

[nridge]

Summary

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

[nridge]

Update a comment

[hokein]

looks mostly good, just a few suggestions on the test.

clang-tools-extra/clangd/FindTarget.cpp

128

nit: use isa.

354

can we have a test for FindTarget? TEST_F(TargetDeclTest, UsingDecl) is an ideal place.

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

738

nit: move to Test(LocateSymbol, Alias).

[nridge]

Address review comments

clang-tools-extra/clangd/FindTarget.cpp

 // for a call to vector<T>::size().
 // 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) {
+    llvm::function_ref<bool(const NamedDecl *ND)> Filter) {
   if (!T)
     return {};
   if (auto *ET = T->getAs<EnumType>()) {
     auto Result =
         ET->getDecl()->lookup(NameFactory(ET->getDecl()->getASTContext()));
     return {Result.begin(), Result.end()};
   }
   if (auto *RD = resolveTypeToRecordDecl(T)) {
     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();
-    });
+    return RD->lookupDependentName(Name, Filter);
   }
   return {};
 }
 
-// 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 auto NonStaticFilter = [](const NamedDecl *D) {
+  return D->isCXXInstanceMember();
+};
+const auto StaticFilter = [](const NamedDecl *D) {
+  return !D->isCXXInstanceMember();
+};
+const auto ValueFilter = [](const NamedDecl *D) {
+  return dyn_cast<ValueDecl>(D) != nullptr;
+};
+
+// 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);
+      NonStaticFilter);
   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
if (const auto *BT = BaseType->getAs<BuiltinType>()) {
       // can get further by analyzing the depedent expression.
       Expr *Base = ME->isImplicitAccess() ? nullptr : ME->getBase();
       if (Base && BT->getKind() == BuiltinType::Dependent) {
         BaseType = resolveExprToType(Base);
       }
     }
     return getMembersReferencedViaDependentName(
         BaseType, [ME](ASTContext &) { return ME->getMember(); },
-        /*IsNonstaticMember=*/true);
+        NonStaticFilter);
   }
   if (const auto *RE = dyn_cast<DependentScopeDeclRefExpr>(E)) {
     return getMembersReferencedViaDependentName(
         RE->getQualifier()->getAsType(),
-        [RE](ASTContext &) { return RE->getDeclName(); },
-        /*IsNonstaticMember=*/false);
+        [RE](ASTContext &) { return RE->getDeclName(); }, StaticFilter);
   }
   if (const auto *CE = dyn_cast<CallExpr>(E)) {
     const auto *CalleeType = resolveExprToType(CE->getCallee());
     if (!CalleeType)
       return {};
     if (const auto *FnTypePtr = CalleeType->getAs<PointerType>())
       CalleeType = FnTypePtr->getPointeeType().getTypePtr();
     if (const FunctionType *FnType = CalleeType->getAs<FunctionType>()) {

 //    There's no Decl `pvec<int>`, we must choose `pvec<X>` or `vector<int*>`
 //    and both are lossy. We must know upfront what the caller ultimately wants.
 //
 // FIXME: improve common dependent scope using name lookup in primary templates.
 // We currently handle DependentScopeDeclRefExpr and
 // CXXDependentScopeMemberExpr, but some other constructs remain to be handled:
 //  - DependentTemplateSpecializationType,
 //  - DependentNameType
-//  - UnresolvedUsingValueDecl
 //  - UnresolvedUsingTypenameDecl
 struct TargetFinder {
   using RelSet = DeclRelationSet;
   using Rel = DeclRelation;
 
 private:
   llvm::SmallDenseMap<const NamedDecl *,
                       std::pair<RelSet, /*InsertionOrder*/ size_t>>
if (const TypedefNameDecl *TND = dyn_cast<TypedefNameDecl>(D)) {
       Flags |= Rel::Alias; // continue with the alias.
     } else if (const UsingDecl *UD = dyn_cast<UsingDecl>(D)) {
       for (const UsingShadowDecl *S : UD->shadows())
         add(S->getUnderlyingDecl(), Flags | Rel::Underlying);
       Flags |= Rel::Alias; // continue with the alias.
     } else if (const auto *NAD = dyn_cast<NamespaceAliasDecl>(D)) {
       add(NAD->getUnderlyingDecl(), Flags | Rel::Underlying);
       Flags |= Rel::Alias; // continue with the alias
+    } else if (const UnresolvedUsingValueDecl *UUVD =
+                   dyn_cast<UnresolvedUsingValueDecl>(D)) {
+      for (const NamedDecl *Target : getMembersReferencedViaDependentName(
+               UUVD->getQualifier()->getAsType(),
+               [UUVD](ASTContext &) { return UUVD->getNameInfo().getName(); },
+               ValueFilter)) {
+        add(Target, Flags | Rel::Underlying);
+      }
+      Flags |= Rel::Alias; // continue with the alias
     } else if (const UsingShadowDecl *USD = dyn_cast<UsingShadowDecl>(D)) {
       // Include the using decl, but don't traverse it. This may end up
       // including *all* shadows, which we don't want.
       report(USD->getUsingDecl(), Flags | Rel::Alias);
       // Shadow decls are synthetic and not themselves interesting.
       // Record the underlying decl instead, if allowed.
       D = USD->getTargetDecl();
       Flags |= Rel::Underlying; // continue with the underlying decl.

clang-tools-extra/clangd/XRefs.cpp

if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
           D->getLocation() == TouchedIdentifier->location()) {
         AddResultDecl(CTSD->getSpecializedTemplate());
         continue;
       }
     }
 
     // Give the underlying decl if navigation is triggered on a non-renaming
     // alias.
-    if (llvm::isa<UsingDecl>(D)) {
+    if (llvm::isa<UsingDecl>(D) || llvm::isa<UnresolvedUsingValueDecl>(D)) {
       // FIXME: address more complicated cases. TargetDecl(... Underlying) gives
       // all overload candidates, we only want the targeted one if the cursor is
       // on an using-alias usage, workround it with getDeclAtPosition.
       llvm::for_each(
           getDeclAtPosition(AST, CurLoc, DeclRelation::Underlying, NodeKind),
           [&](const NamedDecl *UD) { AddResultDecl(UD); });
       continue;
     }

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

const char *Tests[] = {
         @class Foo;
         @interface $decl[[Foo]]
         @end
         @implementation $def[[Foo]]
         @end
         Fo^o * getFoo() {
           return 0;
         }
-      )objc"};
+      )objc",
+      R"cpp(// Member of dependent base
+        template <typename T>
+        struct Base {
+          void [[waldo]]() {}
+        };
+        template <typename T>
+        struct Derived : Base<T> {
+          using Base<T>::w^aldo;
+        };
+      )cpp"};
   for (const char *Test : Tests) {
     Annotations T(Test);
     llvm::Optional<Range> WantDecl;
     llvm::Optional<Range> WantDef;
     if (!T.ranges().empty())
       WantDecl = WantDef = T.range();
     if (!T.ranges("decl").empty())
       WantDecl = T.range("decl");