Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][Index] Use HeuristicResolver in libIndex #125153

Merged

Conversation

HighCommander4
Copy link
Collaborator

@HighCommander4 HighCommander4 commented Jan 31, 2025

The uses replace hand-rolled code that did a subset of what
HeuristicResolver does.

@HighCommander4 HighCommander4 force-pushed the users/HighCommander4/lookupdependentname-libindex branch from ddb04e5 to 656a479 Compare February 17, 2025 02:33
@HighCommander4 HighCommander4 marked this pull request as ready for review February 17, 2025 03:07
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Feb 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 17, 2025

@llvm/pr-subscribers-clang

Author: Nathan Ridge (HighCommander4)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/125153.diff

5 Files Affected:

  • (modified) clang/lib/Index/CMakeLists.txt (+1)
  • (modified) clang/lib/Index/IndexBody.cpp (+16-33)
  • (modified) clang/lib/Index/IndexingContext.cpp (+12)
  • (modified) clang/lib/Index/IndexingContext.h (+7-3)
  • (modified) clang/test/Index/Core/index-dependent-source.cpp (+11-11)
diff --git a/clang/lib/Index/CMakeLists.txt b/clang/lib/Index/CMakeLists.txt
index b4e294304f115..f0d2b579c8df6 100644
--- a/clang/lib/Index/CMakeLists.txt
+++ b/clang/lib/Index/CMakeLists.txt
@@ -23,6 +23,7 @@ add_clang_library(clangIndex
   clangFormat
   clangFrontend
   clangLex
+  clangSema
   clangSerialization
   clangToolingCore
 
diff --git a/clang/lib/Index/IndexBody.cpp b/clang/lib/Index/IndexBody.cpp
index f1dc4d5831ce7..8b4a8889a9e65 100644
--- a/clang/lib/Index/IndexBody.cpp
+++ b/clang/lib/Index/IndexBody.cpp
@@ -13,6 +13,7 @@
 #include "clang/AST/ExprConcepts.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
+#include "clang/Sema/HeuristicResolver.h"
 
 using namespace clang;
 using namespace clang::index;
@@ -168,51 +169,33 @@ class BodyIndexer : public RecursiveASTVisitor<BodyIndexer> {
                                     Parent, ParentDC, Roles, Relations, E);
   }
 
-  bool indexDependentReference(
-      const Expr *E, const Type *T, const DeclarationNameInfo &NameInfo,
-      llvm::function_ref<bool(const NamedDecl *ND)> Filter) {
-    if (!T)
-      return true;
-    const TemplateSpecializationType *TST =
-        T->getAs<TemplateSpecializationType>();
-    if (!TST)
-      return true;
-    TemplateName TN = TST->getTemplateName();
-    const ClassTemplateDecl *TD =
-        dyn_cast_or_null<ClassTemplateDecl>(TN.getAsTemplateDecl());
-    if (!TD)
-      return true;
-    CXXRecordDecl *RD = TD->getTemplatedDecl();
-    if (!RD->hasDefinition())
-      return true;
-    RD = RD->getDefinition();
-    std::vector<const NamedDecl *> Symbols =
-        RD->lookupDependentName(NameInfo.getName(), Filter);
+  bool indexDependentReference(const Expr *E, SourceLocation Loc,
+                               std::vector<const NamedDecl *> TargetSymbols) {
     // FIXME: Improve overload handling.
-    if (Symbols.size() != 1)
+    if (TargetSymbols.size() != 1)
       return true;
-    SourceLocation Loc = NameInfo.getLoc();
     if (Loc.isInvalid())
       Loc = E->getBeginLoc();
     SmallVector<SymbolRelation, 4> Relations;
     SymbolRoleSet Roles = getRolesForRef(E, Relations);
-    return IndexCtx.handleReference(Symbols[0], Loc, Parent, ParentDC, Roles,
-                                    Relations, E);
+    return IndexCtx.handleReference(TargetSymbols[0], Loc, Parent, ParentDC,
+                                    Roles, Relations, E);
   }
 
   bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
-    const DeclarationNameInfo &Info = E->getMemberNameInfo();
-    return indexDependentReference(
-        E, E->getBaseType().getTypePtrOrNull(), Info,
-        [](const NamedDecl *D) { return D->isCXXInstanceMember(); });
+    auto *Resolver = IndexCtx.getResolver();
+    if (!Resolver)
+      return true;
+    return indexDependentReference(E, E->getMemberNameInfo().getLoc(),
+                                   Resolver->resolveMemberExpr(E));
   }
 
   bool VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) {
-    const DeclarationNameInfo &Info = E->getNameInfo();
-    const NestedNameSpecifier *NNS = E->getQualifier();
-    return indexDependentReference(
-        E, NNS->getAsType(), Info,
-        [](const NamedDecl *D) { return !D->isCXXInstanceMember(); });
+    auto *Resolver = IndexCtx.getResolver();
+    if (!Resolver)
+      return true;
+    return indexDependentReference(E, E->getNameInfo().getLoc(),
+                                   Resolver->resolveDeclRefExpr(E));
   }
 
   bool VisitDesignatedInitExpr(DesignatedInitExpr *E) {
diff --git a/clang/lib/Index/IndexingContext.cpp b/clang/lib/Index/IndexingContext.cpp
index 2dd68dfcc5a70..bdd6c5acf1d34 100644
--- a/clang/lib/Index/IndexingContext.cpp
+++ b/clang/lib/Index/IndexingContext.cpp
@@ -14,6 +14,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexDataConsumer.h"
+#include "clang/Sema/HeuristicResolver.h"
 
 using namespace clang;
 using namespace index;
@@ -25,6 +26,17 @@ static bool isGeneratedDecl(const Decl *D) {
   return false;
 }
 
+IndexingContext::IndexingContext(IndexingOptions IndexOpts,
+                                 IndexDataConsumer &DataConsumer)
+    : IndexOpts(IndexOpts), DataConsumer(DataConsumer) {}
+
+IndexingContext::~IndexingContext() = default;
+
+void IndexingContext::setASTContext(ASTContext &ctx) {
+  Ctx = &ctx;
+  Resolver = Ctx ? std::make_unique<HeuristicResolver>(*Ctx) : nullptr;
+}
+
 bool IndexingContext::shouldIndex(const Decl *D) {
   return !isGeneratedDecl(D);
 }
diff --git a/clang/lib/Index/IndexingContext.h b/clang/lib/Index/IndexingContext.h
index 3020b33bea385..01bfcb9d578bc 100644
--- a/clang/lib/Index/IndexingContext.h
+++ b/clang/lib/Index/IndexingContext.h
@@ -21,6 +21,7 @@ namespace clang {
   class Decl;
   class DeclGroupRef;
   class ImportDecl;
+  class HeuristicResolver;
   class TagDecl;
   class TypeSourceInfo;
   class NamedDecl;
@@ -39,15 +40,18 @@ class IndexingContext {
   IndexingOptions IndexOpts;
   IndexDataConsumer &DataConsumer;
   ASTContext *Ctx = nullptr;
+  std::unique_ptr<HeuristicResolver> Resolver;
 
 public:
-  IndexingContext(IndexingOptions IndexOpts, IndexDataConsumer &DataConsumer)
-    : IndexOpts(IndexOpts), DataConsumer(DataConsumer) {}
+  IndexingContext(IndexingOptions IndexOpts, IndexDataConsumer &DataConsumer);
+  ~IndexingContext();
 
   const IndexingOptions &getIndexOpts() const { return IndexOpts; }
   IndexDataConsumer &getDataConsumer() { return DataConsumer; }
 
-  void setASTContext(ASTContext &ctx) { Ctx = &ctx; }
+  void setASTContext(ASTContext &ctx);
+
+  HeuristicResolver *getResolver() const { return Resolver.get(); }
 
   bool shouldIndex(const Decl *D);
 
diff --git a/clang/test/Index/Core/index-dependent-source.cpp b/clang/test/Index/Core/index-dependent-source.cpp
index 8fec9abd1e926..ef414c8fdf7a0 100644
--- a/clang/test/Index/Core/index-dependent-source.cpp
+++ b/clang/test/Index/Core/index-dependent-source.cpp
@@ -3,7 +3,7 @@
 int invalid;
 
 class Base {
-  void baseFunction();
+  void baseFunction() const;
 
   int baseField;
 
@@ -13,7 +13,7 @@ class Base {
 template<typename T>
 class BaseTemplate {
 public:
-  T baseTemplateFunction();
+  T baseTemplateFunction() const;
 
   T baseTemplateField;
 
@@ -25,7 +25,7 @@ class TemplateClass: public Base , public BaseTemplate<T> {
 public:
   ~TemplateClass();
 
-  T function() { }
+  T function() const { }
 
   static void staticFunction() { }
 
@@ -48,27 +48,27 @@ template<typename T, typename S>
 void indexSimpleDependentDeclarations(const TemplateClass<T, S> &object) {
   // Valid instance members:
   object.function();
-// CHECK: [[@LINE-1]]:10 | instance-method/C++ | function | c:@ST>2#T#T@TemplateClass@F@function# | <no-cgname> | Ref,Call,RelCall,RelCont | rel: 1
+// CHECK: [[@LINE-1]]:10 | instance-method/C++ | function | c:@ST>2#T#T@TemplateClass@F@function#1 | <no-cgname> | Ref,Call,RelCall,RelCont | rel: 1
   object.field;
 // CHECK: [[@LINE-1]]:10 | field/C++ | field | c:@ST>2#T#T@TemplateClass@FI@field | <no-cgname> | Ref,RelCont | rel: 1
   object.baseFunction();
-// CHECK: [[@LINE-1]]:10 | instance-method/C++ | baseFunction | c:@S@Base@F@baseFunction# | __ZN4Base12baseFunctionEv | Ref,Call,RelCall,RelCont | rel: 1
+// CHECK: [[@LINE-1]]:10 | instance-method/C++ | baseFunction | c:@S@Base@F@baseFunction#1 | __ZNK4Base12baseFunctionEv | Ref,Call,RelCall,RelCont | rel: 1
   object.baseField;
 // CHECK: [[@LINE-1]]:10 | field/C++ | baseField | c:@S@Base@FI@baseField | <no-cgname> | Ref,RelCont | rel: 1
   object.baseTemplateFunction();
-// CHECK: [[@LINE-1]]:10 | instance-method/C++ | baseTemplateFunction | c:@ST>1#T@BaseTemplate@F@baseTemplateFunction# | <no-cgname> | Ref,Call,RelCall,RelCont | rel: 1
+// CHECK: [[@LINE-1]]:10 | instance-method/C++ | baseTemplateFunction | c:@ST>1#T@BaseTemplate@F@baseTemplateFunction#1 | <no-cgname> | Ref,Call,RelCall,RelCont | rel: 1
   object.baseTemplateField;
 // CHECK: [[@LINE-1]]:10 | field/C++ | baseTemplateField | c:@ST>1#T@BaseTemplate@FI@baseTemplateField | <no-cgname> | Ref,RelCont | rel: 1
 
-  // Invalid instance members:
+  // Static members (these are still valid to access via an instance):
   object.variable;
-// CHECK-NOT: [[@LINE-1]]:10
+// CHECK: [[@LINE-1]]:10 | static-property/C++ | variable | c:@ST>2#T#T@TemplateClass@variable | __ZN13TemplateClass8variableE | Ref,RelCont | rel: 1
   object.staticFunction();
-// CHECK-NOT: [[@LINE-1]]:10
+// CHECK: [[@LINE-1]]:10 | static-method/C++ | staticFunction | c:@ST>2#T#T@TemplateClass@F@staticFunction#S | <no-cgname> | Ref,Call,RelCall,RelCont | rel: 1
   object.Struct;
-// CHECK-NOT: [[@LINE-1]]:10
+// CHECK: [[@LINE-1]]:10 | struct/C | Struct | c:@ST>2#T#T@TemplateClass@S@Struct | <no-cgname> | Ref,RelCont | rel: 1
   object.EnumValue;
-// CHECK-NOT: [[@LINE-1]]:10
+// CHECK: [[@LINE-1]]:10 | enumerator/C | EnumValue | c:@ST>2#T#T@TemplateClass@E@Enum@EnumValue | <no-cgname> | Ref,RelCont | rel: 1
 
   // Valid static members:
   TemplateClass<T, S>::staticFunction();

Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the improvement looks good

The uses replace hand-rolled code that did a subset of what
HeuristicResolver does.
@HighCommander4 HighCommander4 force-pushed the users/HighCommander4/lookupdependentname-libindex branch from 656a479 to 5546172 Compare February 17, 2025 06:10
Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@HighCommander4 HighCommander4 merged commit d09cce1 into main Feb 18, 2025
8 checks passed
@HighCommander4 HighCommander4 deleted the users/HighCommander4/lookupdependentname-libindex branch February 18, 2025 06:13
wldfngrs pushed a commit to wldfngrs/llvm-project that referenced this pull request Feb 19, 2025
The uses replace hand-rolled code that did a subset of what
HeuristicResolver does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants