[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.

352

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 isa<ValueDecl>(D); };
+
+// 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/FindTargetTests.cpp

Code = R"cpp(
     };
     struct Y : X {
       using X::foo;
     };
     int x = Y().[[foo]]();
   )cpp";
   EXPECT_DECLS("MemberExpr", {"using X::foo", Rel::Alias},
                {"int foo()", Rel::Underlying});
+
+  Code = R"cpp(
+      template <typename T>
+      struct Base {
+        void waldo() {}
+      };
+      template <typename T>
+      struct Derived : Base<T> {
+        using Base<T>::[[waldo]];
+      };
+    )cpp";
+  EXPECT_DECLS("UnresolvedUsingValueDecl", {"using Base<T>::waldo", Rel::Alias},
+               {"void waldo()", Rel::Underlying});
 }
 
 TEST_F(TargetDeclTest, ConstructorInitList) {
   Code = R"cpp(
     struct X {
       int a;
       X() : [[a]](42) {}
     };

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

const char *Tests[] = {
         @interface $decl[[Foo]]
         @end
         @implementation $def[[Foo]]
         @end
         Fo^o * getFoo() {
           return 0;
         }
       )objc"};
   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");
     if (!T.ranges("def").empty())
TEST(LocateSymbol, TextualDependent) {
   EXPECT_THAT(Results,
               UnorderedElementsAre(
                   Sym("uniqueMethodName", Header.range("FooLoc"), llvm::None),
                   Sym("uniqueMethodName", Header.range("BarLoc"), llvm::None)));
 }
 
 TEST(LocateSymbol, Alias) {
   const char *Tests[] = {
       R"cpp(
       template <class T> struct function {};
       template <class T> using [[callback]] = function<T()>;
 
       c^allback<int> foo;
     )cpp",
 
       // triggered on non-definition of a renaming alias: should not give any
       // underlying decls.
       R"cpp(
       class Foo {};
       typedef Foo [[Bar]];
 
       B^ar b;
     )cpp",
       R"cpp(
       class Foo {};
       using [[Bar]] = Foo; // definition
       Ba^r b;
     )cpp",
 
       // triggered on the underlying decl of a renaming alias.
       R"cpp(
       class [[Foo]];
       using Bar = Fo^o;
     )cpp",
 
       // triggered on definition of a non-renaming alias: should give underlying
       // decls.
       R"cpp(
       namespace ns { class [[Foo]] {}; }
       using ns::F^oo;
     )cpp",
 
       R"cpp(
       namespace ns { int [[x]](char); int [[x]](double); }
       using ns::^x;
     )cpp",
 
       R"cpp(
       namespace ns { int [[x]](char); int x(double); }
       using ns::x;
       int y = ^x('a');
     )cpp",
 
       R"cpp(
       namespace ns { class [[Foo]] {}; }
       using ns::Foo;
       F^oo f;
     )cpp",
 
       // other cases that don't matter much.
       R"cpp(
       class Foo {};
       typedef Foo [[Ba^r]];
     )cpp",
       R"cpp(
       class Foo {};
       using [[B^ar]] = Foo;
     )cpp",
+
+      // Member of dependent base
+      R"cpp(
+      template <typename T>
+      struct Base {
+        void [[waldo]]() {}
+      };
+      template <typename T>
+      struct Derived : Base<T> {
+        using Base<T>::w^aldo;
+      };
+    )cpp",
   };
 
   for (const auto* Case : Tests) {
     SCOPED_TRACE(Case);
     auto T = Annotations(Case);
     auto AST = TestTU::withCode(T.code()).build();
     EXPECT_THAT(locateSymbolAt(AST, T.point()),
                 ::testing::UnorderedPointwise(DeclRange(), T.ranges()));