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 IndexTypeSourceInfo as well #128106

Merged

Conversation

HighCommander4
Copy link
Collaborator

No description provided.

@HighCommander4 HighCommander4 force-pushed the users/HighCommander4/lookupdependentname-libindex2 branch from 10edf87 to 84fac9a Compare February 21, 2025 02:16
@HighCommander4 HighCommander4 marked this pull request as ready for review February 21, 2025 04:03
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Feb 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 21, 2025

@llvm/pr-subscribers-clang

Author: Nathan Ridge (HighCommander4)

Changes

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

1 Files Affected:

  • (modified) clang/lib/Index/IndexTypeSourceInfo.cpp (+3-21)
diff --git a/clang/lib/Index/IndexTypeSourceInfo.cpp b/clang/lib/Index/IndexTypeSourceInfo.cpp
index b986ccde57452..d5d0a3c422871 100644
--- a/clang/lib/Index/IndexTypeSourceInfo.cpp
+++ b/clang/lib/Index/IndexTypeSourceInfo.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Sema/HeuristicResolver.h"
 #include "llvm/ADT/ScopeExit.h"
 
 using namespace clang;
@@ -207,27 +208,8 @@ class TypeIndexer : public RecursiveASTVisitor<TypeIndexer> {
   }
 
   bool VisitDependentNameTypeLoc(DependentNameTypeLoc TL) {
-    const DependentNameType *DNT = TL.getTypePtr();
-    const NestedNameSpecifier *NNS = DNT->getQualifier();
-    const Type *T = NNS->getAsType();
-    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();
-    DeclarationName Name(DNT->getIdentifier());
-    std::vector<const NamedDecl *> Symbols = RD->lookupDependentName(
-        Name, [](const NamedDecl *ND) { return isa<TypeDecl>(ND); });
+    std::vector<const NamedDecl *> Symbols =
+        IndexCtx.getResolver()->resolveDependentNameType(TL.getTypePtr());
     if (Symbols.size() != 1)
       return true;
     return IndexCtx.handleReference(Symbols[0], TL.getNameLoc(), Parent,

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, this is a nice improvement.

Nit: I think it'd be better to say NFC in the PR title.

@HighCommander4
Copy link
Collaborator Author

Nit: I think it'd be better to say NFC in the PR title.

Technically, this is a functional change since HeuristicResolver::resolveDependentNameType() does a bit more than the code it's replacing here; as a result, libIndex will be able to resolve a DependentNameType to a target declaration in more cases.

I considered constructing a test case to demonstrate, but given that HeuristicResolver has its own fairly extensive test suite, I'm not sure if it makes sense to duplicate that effort for each downstream feature that uses it.

@HighCommander4 HighCommander4 merged commit fd5d1cb into main Feb 21, 2025
14 checks passed
@HighCommander4 HighCommander4 deleted the users/HighCommander4/lookupdependentname-libindex2 branch February 21, 2025 05:45
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 21, 2025

LLVM Buildbot has detected a new failure on builder cross-project-tests-sie-ubuntu-dwarf5 running on doug-worker-1b while building clang at step 2 "checkout".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/163/builds/13886

Here is the relevant piece of the build log for the reference
Step 2 (checkout) failure: update (failure)
...
 * [new tag]                   llvmorg-5.0.1-rc1  -> llvmorg-5.0.1-rc1
 * [new tag]                   llvmorg-5.0.1-rc2  -> llvmorg-5.0.1-rc2
 * [new tag]                   llvmorg-5.0.1-rc3  -> llvmorg-5.0.1-rc3
 * [new tag]                   llvmorg-5.0.2      -> llvmorg-5.0.2
 * [new tag]                   llvmorg-5.0.2-rc1  -> llvmorg-5.0.2-rc1
 * [new tag]                   llvmorg-5.0.2-rc2  -> llvmorg-5.0.2-rc2
 * [new tag]                   llvmorg-6.0.0      -> llvmorg-6.0.0
 * [new tag]                   llvmorg-6.0.0-rc1  -> llvmorg-6.0.0-rc1
 * [new tag]                   llvmorg-6.0.0-rc2  -> llvmorg-6.0.0-rc2
 * [new tag]                   llvmorg-6.0.0-rc3  -> llvmorg-6.0.0-rc3
 * [new tag]                   llvmorg-6.0.1      -> llvmorg-6.0.1
 * [new tag]                   llvmorg-6.0.1-rc1  -> llvmorg-6.0.1-rc1
 * [new tag]                   llvmorg-6.0.1-rc2  -> llvmorg-6.0.1-rc2
 * [new tag]                   llvmorg-6.0.1-rc3  -> llvmorg-6.0.1-rc3
 * [new tag]                   llvmorg-7.0.0      -> llvmorg-7.0.0
 * [new tag]                   llvmorg-7.0.0-rc1  -> llvmorg-7.0.0-rc1
 * [new tag]                   llvmorg-7.0.0-rc2  -> llvmorg-7.0.0-rc2
 * [new tag]                   llvmorg-7.0.0-rc3  -> llvmorg-7.0.0-rc3
 * [new tag]                   llvmorg-7.0.1      -> llvmorg-7.0.1
 * [new tag]                   llvmorg-7.0.1-rc1  -> llvmorg-7.0.1-rc1
 * [new tag]                   llvmorg-7.0.1-rc2  -> llvmorg-7.0.1-rc2
 * [new tag]                   llvmorg-7.0.1-rc3  -> llvmorg-7.0.1-rc3
 * [new tag]                   llvmorg-7.1.0      -> llvmorg-7.1.0
 * [new tag]                   llvmorg-7.1.0-rc1  -> llvmorg-7.1.0-rc1
 * [new tag]                   llvmorg-8.0.0      -> llvmorg-8.0.0
 * [new tag]                   llvmorg-8.0.0-rc1  -> llvmorg-8.0.0-rc1
 * [new tag]                   llvmorg-8.0.0-rc2  -> llvmorg-8.0.0-rc2
 * [new tag]                   llvmorg-8.0.0-rc3  -> llvmorg-8.0.0-rc3
 * [new tag]                   llvmorg-8.0.0-rc4  -> llvmorg-8.0.0-rc4
 * [new tag]                   llvmorg-8.0.0-rc5  -> llvmorg-8.0.0-rc5
 * [new tag]                   llvmorg-8.0.1      -> llvmorg-8.0.1
 * [new tag]                   llvmorg-8.0.1-rc1  -> llvmorg-8.0.1-rc1
 * [new tag]                   llvmorg-8.0.1-rc2  -> llvmorg-8.0.1-rc2
 * [new tag]                   llvmorg-8.0.1-rc3  -> llvmorg-8.0.1-rc3
 * [new tag]                   llvmorg-8.0.1-rc4  -> llvmorg-8.0.1-rc4
 * [new tag]                   llvmorg-9.0.0      -> llvmorg-9.0.0
 * [new tag]                   llvmorg-9.0.0-rc1  -> llvmorg-9.0.0-rc1
 * [new tag]                   llvmorg-9.0.0-rc2  -> llvmorg-9.0.0-rc2
 * [new tag]                   llvmorg-9.0.0-rc3  -> llvmorg-9.0.0-rc3
 * [new tag]                   llvmorg-9.0.0-rc4  -> llvmorg-9.0.0-rc4
 * [new tag]                   llvmorg-9.0.0-rc5  -> llvmorg-9.0.0-rc5
 * [new tag]                   llvmorg-9.0.0-rc6  -> llvmorg-9.0.0-rc6
 * [new tag]                   llvmorg-9.0.1      -> llvmorg-9.0.1
 * [new tag]                   llvmorg-9.0.1-rc1  -> llvmorg-9.0.1-rc1
 * [new tag]                   llvmorg-9.0.1-rc2  -> llvmorg-9.0.1-rc2
 * [new tag]                   llvmorg-9.0.1-rc3  -> llvmorg-9.0.1-rc3
fatal: reference is not a tree: fd5d1cbb75e4278d9074ff105efd3ab48f778b4b
From https://github.com/llvm/llvm-project
 * branch                      main       -> FETCH_HEAD
fatal: reference is not a tree: fd5d1cbb75e4278d9074ff105efd3ab48f778b4b

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.

4 participants