fix: Improve approximate dependent method lookup (#321)

Author: varungandhi-src

indexer/ApproximateNameResolver.cc

@@ -0,0 +1,98 @@
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/DeclarationName.h"
+#include "clang/AST/Type.h"
+#include "llvm/ADT/SmallVector.h"
+
+#include "indexer/ApproximateNameResolver.h"
+#include "indexer/Enforce.h"
+
+#include "spdlog/spdlog.h"
+
+namespace scip_clang {
+
+MemberLookupKey::MemberLookupKey(const clang::Type *type,
+                                 const clang::DeclarationNameInfo &declNameInfo)
+    : canonicalTypePtr(type->getCanonicalTypeInternal().getTypePtrOrNull()),
+      declarationName(declNameInfo.getName().getAsOpaqueInteger()) {
+  ENFORCE(this->canonicalTypePtr,
+          "member lookups should exit early for null base type");
+}
+
+llvm::SmallVector<const clang::NamedDecl *, 1>
+ApproximateNameResolver::tryResolveMember(
+    const clang::Type *type, const clang::DeclarationNameInfo &declNameInfo) {
+  llvm::SmallVector<const clang::NamedDecl *, 1> results;
+  if (!type) {
+    return results;
+  }
+  MemberLookupKey key{type, declNameInfo};
+  auto it = this->dependentNameLookupCache.find(key);
+  if (it != this->dependentNameLookupCache.end()) {
+    return it->second;
+  }
+
+  auto filter = [](const clang::NamedDecl *namedDecl) -> bool {
+    return !llvm::isa<clang::UsingDecl>(namedDecl);
+  };
+
+  llvm::SmallVector<const clang::Type *, 2> typesToLookup;
+  typesToLookup.push_back(type);
+
+  while (!typesToLookup.empty()) {
+    auto *type = typesToLookup.back();
+    typesToLookup.pop_back();
+    auto *cxxRecordDecl = Self::tryFindDeclForType(type);
+    if (!cxxRecordDecl) {
+      continue;
+    }
+    auto lookupResults =
+        cxxRecordDecl->lookupDependentName(declNameInfo.getName(), filter);
+    for (auto *namedDecl : lookupResults) {
+      auto *unresolvedUsingValueDecl =
+          llvm::dyn_cast<clang::UnresolvedUsingValueDecl>(namedDecl);
+      if (!unresolvedUsingValueDecl) {
+        results.push_back(namedDecl);
+        continue;
+      }
+      if (auto *nestedNameSpecifier =
+              unresolvedUsingValueDecl->getQualifier()) {
+        if (auto *innerType = nestedNameSpecifier->getAsType()) {
+          typesToLookup.push_back(innerType);
+        }
+      }
+    }
+  }
+
+  this->dependentNameLookupCache.insert({key, results});
+  return results;
+}
+
+// static
+clang::CXXRecordDecl *
+ApproximateNameResolver::tryFindDeclForType(const clang::Type *type) {
+  if (const auto *recordType = type->getAs<clang::RecordType>())
+    return llvm::dyn_cast<clang::CXXRecordDecl>(recordType->getDecl());
+
+  if (const auto *injectedClassNameType =
+          type->getAs<clang::InjectedClassNameType>())
+    type = injectedClassNameType->getInjectedSpecializationType()
+               .getTypePtrOrNull();
+  if (!type)
+    return nullptr;
+
+  const auto *templateSpecializationType =
+      type->getAs<clang::TemplateSpecializationType>();
+  if (!templateSpecializationType)
+    return nullptr;
+
+  auto *templateDecl = dyn_cast_or_null<clang::ClassTemplateDecl>(
+      templateSpecializationType->getTemplateName().getAsTemplateDecl());
+  if (!templateDecl)
+    return nullptr;
+
+  return templateDecl->getTemplatedDecl();
+}
+
+} // namespace scip_clang

indexer/ApproximateNameResolver.h

@@ -0,0 +1,67 @@
+#ifndef SCIP_CLANG_APPROX_NAME_RESOLVER
+#define SCIP_CLANG_APPROX_NAME_RESOLVER
+
+#include <utility>
+
+#include "absl/container/flat_hash_map.h"
+
+#include "llvm/ADT/SmallVector.h"
+
+#include "indexer/Derive.h"
+
+namespace clang {
+class ASTContext;
+class CXXRecordDecl;
+class DeclarationNameInfo;
+class NamedDecl;
+class QualType;
+class Type;
+} // namespace clang
+
+namespace scip_clang {
+
+class MemberLookupKey {
+  const clang::Type *canonicalTypePtr;
+  uintptr_t declarationName;
+
+public:
+  MemberLookupKey(const clang::Type *, const clang::DeclarationNameInfo &);
+
+  template <typename H>
+  friend H AbslHashValue(H h, const MemberLookupKey &self) {
+    return H::combine(std::move(h), (void *)self.canonicalTypePtr,
+                      self.declarationName);
+  }
+
+  DERIVE_CMP_ALL(MemberLookupKey)
+};
+
+/// Type similar to clangd's HeuristicResolver, used for performing
+/// best-effort name resolution when encountered unresolved names.
+///
+/// We don't directly reuse the code from clangd, since it's not that
+/// much code (<300 lines), and having own code let's us evolve it
+/// independently and add different heuristics.
+///
+/// (Named differently to reduce risk of confusion.)
+class ApproximateNameResolver {
+  [[maybe_unused]] const clang::ASTContext &astContext; // for debugging
+  using Self = ApproximateNameResolver;
+  using ResultVec = llvm::SmallVector<const clang::NamedDecl *, 1>;
+  absl::flat_hash_map<MemberLookupKey, ResultVec> dependentNameLookupCache;
+
+public:
+  ApproximateNameResolver(const clang::ASTContext &astContext)
+      : astContext(astContext), dependentNameLookupCache() {}
+
+  ResultVec tryResolveMember(const clang::Type *,
+                             const clang::DeclarationNameInfo &);
+
+private:
+  // Analog to HeuristicResolver.cc's resolveTypeToRecordDecl
+  static clang::CXXRecordDecl *tryFindDeclForType(const clang::Type *);
+};
+
+} // namespace scip_clang
+
+#endif // SCIP_CLANG_APPROX_NAME_RESOLVER

indexer/Indexer.cc

@@ -27,6 +27,7 @@
 #include "scip/scip.pb.h"
 
 #include "indexer/AbslExtras.h"
+#include "indexer/ApproximateNameResolver.h"
 #include "indexer/DebugHelpers.h"
 #include "indexer/Enforce.h"
 #include "indexer/Indexer.h"
@@ -306,7 +307,8 @@ TuIndexer::TuIndexer(const clang::SourceManager &sourceManager,
                      SymbolFormatter &symbolFormatter,
                      GetStableFileId getStableFileId)
     : sourceManager(sourceManager), langOptions(langOptions),
-      astContext(astContext), symbolFormatter(symbolFormatter), documentMap(),
+      astContext(astContext), symbolFormatter(symbolFormatter),
+      approximateNameResolver(astContext), documentMap(),
       getStableFileId(getStableFileId), externalSymbols(),
       forwardDeclarations() {}
 
@@ -891,21 +893,14 @@ void TuIndexer::trySaveMemberReferenceViaLookup(
   if (baseType.isNull()) {
     return;
   }
-  clang::QualType actualBaseType = baseType.getCanonicalType();
+  auto derefBaseType = baseType.getCanonicalType();
   // C++23's 'deducing this' feature allows the base type to be a reference
   if (baseType->isPointerType() || baseType->isReferenceType()) {
-    actualBaseType = baseType->getPointeeType();
+    derefBaseType = baseType->getPointeeType();
   }
-  auto *recordDecl = actualBaseType->getAsCXXRecordDecl();
-  if (!recordDecl) {
-    return;
-  }
-  // Filter out UsingDecl as the corresponding UsingShadowDecl is sufficient.
-  auto lookupResult = recordDecl->lookupDependentName(
-      memberNameInfo.getName(), [&](const clang::NamedDecl *decl) -> bool {
-        return !llvm::isa<clang::UsingDecl>(decl);
-      });
-  for (auto *namedDecl : lookupResult) {
+  auto namedDecls = this->approximateNameResolver.tryResolveMember(
+      derefBaseType.getTypePtrOrNull(), memberNameInfo);
+  for (auto *namedDecl : namedDecls) {
     auto optSymbol = this->symbolFormatter.getNamedDeclSymbol(*namedDecl);
     if (optSymbol) {
       this->saveReference(*optSymbol, memberNameInfo.getLoc());

indexer/Indexer.h

@@ -16,6 +16,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/SmallVector.h"
 
+#include "indexer/ApproximateNameResolver.h"
 #include "indexer/ClangAstMacros.h"
 #include "indexer/Comparison.h"
 #include "indexer/Derive.h"
@@ -39,6 +40,7 @@ class DeclarationNameInfo;
 class LangOptions;
 class MacroDefinition;
 class MacroInfo;
+class NamedDecl;
 class NestedNameSpecifierLoc;
 class QualType;
 class SourceManager;
@@ -234,6 +236,8 @@ class TuIndexer final {
   const clang::LangOptions &langOptions;
   [[maybe_unused]] const clang::ASTContext &astContext;
   SymbolFormatter &symbolFormatter;
+  ApproximateNameResolver approximateNameResolver;
+
   absl::flat_hash_map<llvm_ext::AbslHashAdapter<clang::FileID>, PartialDocument>
       documentMap;
   GetStableFileId getStableFileId;

test/index/functions/templates.cc

@@ -40,3 +40,22 @@ void test_template() {
   h1<int>(0);
   h1<char>(0);
 }
+
+template <typename T>
+struct Q0 {
+  void f() {}
+};
+
+template <typename T>
+struct Q1: Q0<T> {
+  using Base1 = Q0<T>;
+  using Base1::f;
+  void g() { f(); }
+};
+
+template <typename T>
+struct Q2: Q1<T> {
+  using Base2 = Q1<T>;
+  using Base2::f;
+  void h() { f(); }
+};

test/index/functions/templates.snapshot.cc

@@ -94,3 +94,45 @@
     h1<char>(0);
 //  ^^ reference [..] h1(9b289cee16747614).
   }
+  
+  template <typename T>
+//                   ^ definition local 11
+  struct Q0 {
+//       ^^ definition [..] Q0#
+    void f() {}
+//       ^ definition [..] Q0#f(49f6e7a06ebc5aa8).
+  };
+  
+  template <typename T>
+//                   ^ definition local 12
+  struct Q1: Q0<T> {
+//       ^^ definition [..] Q1#
+//           ^^ reference [..] Q0#
+//              ^ reference local 12
+    using Base1 = Q0<T>;
+//        ^^^^^ definition [..] Q1#Base1#
+//                ^^ reference [..] Q0#
+//                   ^ reference local 12
+    using Base1::f;
+//        ^^^^^ reference [..] Q1#Base1#
+    void g() { f(); }
+//       ^ definition [..] Q1#g(49f6e7a06ebc5aa8).
+//             ^ reference [..] Q0#f(49f6e7a06ebc5aa8).
+  };
+  
+  template <typename T>
+//                   ^ definition local 13
+  struct Q2: Q1<T> {
+//       ^^ definition [..] Q2#
+//           ^^ reference [..] Q1#
+//              ^ reference local 13
+    using Base2 = Q1<T>;
+//        ^^^^^ definition [..] Q2#Base2#
+//                ^^ reference [..] Q1#
+//                   ^ reference local 13
+    using Base2::f;
+//        ^^^^^ reference [..] Q2#Base2#
+    void h() { f(); }
+//       ^ definition [..] Q2#h(49f6e7a06ebc5aa8).
+//             ^ reference [..] Q0#f(49f6e7a06ebc5aa8).
+  };