[clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

[nridge]

Summary

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

[nridge]

Depends on D152500

[nridge]

clang-tools-extra/clangd/HeuristicResolver.cpp

38

I wanted to call out the change on this line which is easy to overlook due to the code move:

We were previously calling TypeDecl::getTypeForDecl(), but that's actually a low-level accessor for a cached value. The function that also populates the value if needed is ASTContext::getTypeDeclType(), hence I switch to using that.

[hokein]

clang-tools-extra/clangd/HeuristicResolver.cpp

33

the reason to promote this utility function to a class method seems to be able to access the member field ASTContext. If so, I'd probably just passing the ASTContext as a parameter rather then making them member methods.

38

thanks for the highlight, I think this change makes sense.

60

Is the resolveDeclsToType call necessary here?
The code path we're doing is dependent-type => Decl* => Type => Decl*, the later two steps seems redundant, can we just use the Decl result from resolveDependentNameType()?

[nridge]

Address review comments

clang-tools-extra/clangd/HeuristicResolver.cpp

60

The Decl returned by resolveDependentNameType() in this testcase is a TypeAliasDecl. The function needs to return the CXXRecordDecl which is named by the target type of the alias.

We could add logic to "desugar" the TypeAliasDecl, but it seems easier to reuse the existing logic that does the desugaring in type-land.

[hokein]

clang-tools-extra/clangd/HeuristicResolver.cpp

33

nit: move this helper function to anonymous namespace.

60

I see, thanks for the explanation (this part of code is a bit hard to follow).

[nridge]

Address last comment

clang-tools-extra/clangd/HeuristicResolver.h

private:
   std::vector<const NamedDecl *> resolveDependentMember(
       const Type *T, DeclarationName Name,
       llvm::function_ref<bool(const NamedDecl *ND)> Filter) const;
 
   // Try to heuristically resolve the type of a possibly-dependent expression
   // `E`.
   const Type *resolveExprToType(const Expr *E) const;
   std::vector<const NamedDecl *> resolveExprToDecls(const Expr *E) const;
+
+  // Helper function for HeuristicResolver::resolveDependentMember()
+  // which takes a possibly-dependent type `T` and heuristically
+  // resolves it to a CXXRecordDecl in which we can try name lookup.
+  CXXRecordDecl *resolveTypeToRecordDecl(const Type *T) const;
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/HeuristicResolver.cpp

 //===--- HeuristicResolver.cpp ---------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "HeuristicResolver.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/Type.h"
 
 namespace clang {
 namespace clangd {
 
 // Convenience lambdas for use as the 'Filter' parameter of
 // HeuristicResolver::resolveDependentMember().
 const auto NoFilter = [](const NamedDecl *D) { return true; };
 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); };
 const auto TypeFilter = [](const NamedDecl *D) { return isa<TypeDecl>(D); };
 const auto TemplateFilter = [](const NamedDecl *D) {
   return isa<TemplateDecl>(D);
 };
 
+namespace {
+
+const Type *resolveDeclsToType(const std::vector<const NamedDecl *> &Decls,
+                               ASTContext &Ctx) {
+  if (Decls.size() != 1) // Names an overload set -- just bail.
+    return nullptr;
+  if (const auto *TD = dyn_cast<TypeDecl>(Decls[0])) {
+    return Ctx.getTypeDeclType(TD).getTypePtr();
+  }
+  if (const auto *VD = dyn_cast<ValueDecl>(Decls[0])) {
+    return VD->getType().getTypePtrOrNull();
+  }
+  return nullptr;
+}
+
+} // namespace
+
 // Helper function for HeuristicResolver::resolveDependentMember()
 // which takes a possibly-dependent type `T` and heuristically
 // resolves it to a CXXRecordDecl in which we can try name lookup.
-CXXRecordDecl *resolveTypeToRecordDecl(const Type *T) {
+CXXRecordDecl *HeuristicResolver::resolveTypeToRecordDecl(const Type *T) const {
   assert(T);
 
   // Unwrap type sugar such as type aliases.
   T = T->getCanonicalTypeInternal().getTypePtr();
 
+  if (const auto *DNT = T->getAs<DependentNameType>()) {
+    T = resolveDeclsToType(resolveDependentNameType(DNT), Ctx);
+    if (!T)
+      return nullptr;
+    T = T->getCanonicalTypeInternal().getTypePtr();
+  }
+
   if (const auto *RT = T->getAs<RecordType>())
     return dyn_cast<CXXRecordDecl>(RT->getDecl());
 
   if (const auto *ICNT = T->getAs<InjectedClassNameType>())
     T = ICNT->getInjectedSpecializationType().getTypePtrOrNull();
   if (!T)
     return nullptr;
 

 std::vector<const NamedDecl *>
 HeuristicResolver::resolveTemplateSpecializationType(
     const DependentTemplateSpecializationType *DTST) const {
   return resolveDependentMember(
       resolveNestedNameSpecifierToType(DTST->getQualifier()),
       DTST->getIdentifier(), TemplateFilter);
 }
 
-const Type *resolveDeclsToType(const std::vector<const NamedDecl *> &Decls) {
-  if (Decls.size() != 1) // Names an overload set -- just bail.
-    return nullptr;
-  if (const auto *TD = dyn_cast<TypeDecl>(Decls[0])) {
-    return TD->getTypeForDecl();
-  }
-  if (const auto *VD = dyn_cast<ValueDecl>(Decls[0])) {
-    return VD->getType().getTypePtrOrNull();
-  }
-  return nullptr;
-}
-
 std::vector<const NamedDecl *>
 HeuristicResolver::resolveExprToDecls(const Expr *E) const {
   if (const auto *ME = dyn_cast<CXXDependentScopeMemberExpr>(E)) {
     return resolveMemberExpr(ME);
   }
   if (const auto *RE = dyn_cast<DependentScopeDeclRefExpr>(E)) {
     return resolveDeclRefExpr(RE);
   }
   if (const auto *OE = dyn_cast<OverloadExpr>(E)) {
     return {OE->decls_begin(), OE->decls_end()};
   }
   if (const auto *CE = dyn_cast<CallExpr>(E)) {
     return resolveTypeOfCallExpr(CE);
   }
   if (const auto *ME = dyn_cast<MemberExpr>(E))
     return {ME->getMemberDecl()};
 
   return {};
 }
 
 const Type *HeuristicResolver::resolveExprToType(const Expr *E) const {
   std::vector<const NamedDecl *> Decls = resolveExprToDecls(E);
   if (!Decls.empty())
-    return resolveDeclsToType(Decls);
+    return resolveDeclsToType(Decls, Ctx);
 
   return E->getType().getTypePtr();
 }
 
 const Type *HeuristicResolver::resolveNestedNameSpecifierToType(
     const NestedNameSpecifier *NNS) const {
   if (!NNS)
     return nullptr;
 
   // The purpose of this function is to handle the dependent (Kind ==
   // Identifier) case, but we need to recurse on the prefix because
   // that may be dependent as well, so for convenience handle
   // the TypeSpec cases too.
   switch (NNS->getKind()) {
   case NestedNameSpecifier::TypeSpec:
   case NestedNameSpecifier::TypeSpecWithTemplate:
     return NNS->getAsType();
   case NestedNameSpecifier::Identifier: {
-    return resolveDeclsToType(resolveDependentMember(
+    return resolveDeclsToType(
+        resolveDependentMember(
             resolveNestedNameSpecifierToType(NNS->getPrefix()),
-        NNS->getAsIdentifier(), TypeFilter));
+            NNS->getAsIdentifier(), TypeFilter),
+        Ctx);
   }
   default:
     break;
   }
   return nullptr;
 }
 
 std::vector<const NamedDecl *> HeuristicResolver::resolveDependentMember(

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

Code = R"cpp(
         template <typename T>
         using Wally = Waldo<T>;
         template <typename T>
         void foo(Wally<T> w) {
           w.[[find]]();
         }
   )cpp";
   EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()");
+
+  Code = R"cpp(
+        template <typename T>
+        struct Waldo {
+          void find();
+        };
+        template <typename T>
+        struct MetaWaldo {
+          using Type = Waldo<T>;
+        };
+        template <typename T>
+        void foo(typename MetaWaldo<T>::Type w) {
+          w.[[find]]();
+        }
+  )cpp";
+  EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()");
 }
 
 TEST_F(TargetDeclTest, DependentTypes) {
   // Heuristic resolution of dependent type name
   Code = R"cpp(
         template <typename>
         struct A { struct B {}; };