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

HighCommander4 · 2025-01-31T02:32:50Z

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

llvmbot · 2025-02-17T03:07:54Z

@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();

zyn0217

Thanks, the improvement looks good

clang/lib/Index/IndexBody.cpp

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

zyn0217

LGTM

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

shafik · 2025-05-06T00:46:07Z

clang/lib/Index/IndexingContext.cpp

+                                 IndexDataConsumer &DataConsumer)
+    : IndexOpts(IndexOpts), DataConsumer(DataConsumer) {}
+
+IndexingContext::~IndexingContext() = default;


@HighCommander4 you defaulted the destructor in the source file b/c of the std::unique_ptr<HeuristicResolver> Resolver which would require the HeuristicResolver to be complete when the destructor is defined, correct?

This was flagged by static analysis as a rule of three violation but it looks like it makes sense in this case but it should have been explicitly documented with a comment b/c it is not obvious w/o some digging.

That's right; the motivation is to avoid unnecessary header dependencies (e.g. IndexingContext.h including HeuristicResolver.h), to limit incremental rebuild times when a header is modified.

I'm happy to add a comment.

Sent out #138640

HighCommander4 force-pushed the users/HighCommander4/lookupdependentname-libindex branch from ddb04e5 to 656a479 Compare February 17, 2025 02:33

HighCommander4 marked this pull request as ready for review February 17, 2025 03:07

llvmbot added the clang Clang issues not falling into any other category label Feb 17, 2025

HighCommander4 requested review from hokein and zyn0217 February 17, 2025 03:07

zyn0217 reviewed Feb 17, 2025

View reviewed changes

clang/lib/Index/IndexBody.cpp Outdated Show resolved Hide resolved

[clang][Index] Use HeuristicResolver in libIndex

5546172

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

HighCommander4 force-pushed the users/HighCommander4/lookupdependentname-libindex branch from 656a479 to 5546172 Compare February 17, 2025 06:10

zyn0217 approved these changes Feb 17, 2025

View reviewed changes

HighCommander4 merged commit d09cce1 into main Feb 18, 2025
8 checks passed

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

[clang][Index] Use HeuristicResolver in libIndex (llvm#125153)

3153708

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

HighCommander4 mentioned this pull request Feb 23, 2025

[clang][NFC] Remove CXXRecordDecl::lookupDependentName() and its helpers #128392

Merged

shafik reviewed May 6, 2025

View reviewed changes

HighCommander4 mentioned this pull request May 6, 2025

[clang][Index] Add comment about out-of-line defaulted destructor #138640

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

HighCommander4 commented Jan 31, 2025 •

edited

Loading

Uh oh!

llvmbot commented Feb 17, 2025

Uh oh!

zyn0217 left a comment

Uh oh!

Uh oh!

zyn0217 left a comment

Uh oh!

Uh oh!

shafik May 6, 2025

Uh oh!

HighCommander4 May 6, 2025

Uh oh!

HighCommander4 May 6, 2025

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

HighCommander4 commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 17, 2025

Uh oh!

zyn0217 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zyn0217 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shafik May 6, 2025

Choose a reason for hiding this comment

Uh oh!

HighCommander4 May 6, 2025

Choose a reason for hiding this comment

Uh oh!

HighCommander4 May 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HighCommander4 commented Jan 31, 2025 •

edited

Loading