https://github.com/timon-ul updated https://github.com/llvm/llvm-project/pull/169742
>From 44ebad6933fccab5bcfc866ee56dd5381da5f452 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Mon, 10 Nov 2025 10:40:14 +0100 Subject: [PATCH 01/13] clangd: make forwarding heuristic available for more locations --- clang-tools-extra/clangd/AST.cpp | 21 +++++++++++++++++++++ clang-tools-extra/clangd/AST.h | 4 ++++ clang-tools-extra/clangd/Preamble.cpp | 22 +--------------------- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 0dcff2eae05e7..4b73bdba8aaa9 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -1040,5 +1040,26 @@ bool isExpandedFromParameterPack(const ParmVarDecl *D) { return getUnderlyingPackType(D) != nullptr; } +bool isLikelyForwardingFunction(FunctionTemplateDecl *FT) { + const auto *FD = FT->getTemplatedDecl(); + const auto NumParams = FD->getNumParams(); + // Check whether its last parameter is a parameter pack... + if (NumParams > 0) { + const auto *LastParam = FD->getParamDecl(NumParams - 1); + if (const auto *PET = dyn_cast<PackExpansionType>(LastParam->getType())) { + // ... of the type T&&... or T... + const auto BaseType = PET->getPattern().getNonReferenceType(); + if (const auto *TTPT = + dyn_cast<TemplateTypeParmType>(BaseType.getTypePtr())) { + // ... whose template parameter comes from the function directly + if (FT->getTemplateParameters()->getDepth() == TTPT->getDepth()) { + return true; + } + } + } + } + return false; +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 2b83595e5b8e9..af45ae2d9022d 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -253,6 +253,10 @@ resolveForwardingParameters(const FunctionDecl *D, unsigned MaxDepth = 10); /// reference to one (e.g. `Args&...` or `Args&&...`). bool isExpandedFromParameterPack(const ParmVarDecl *D); +/// Heuristic that checks if FT is forwarding a parameter pack to another +/// function. (e.g. `make_unique`). +bool isLikelyForwardingFunction(FunctionTemplateDecl *FT); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index 8af9e4649218d..09aaf3290b585 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "Preamble.h" +#include "AST.h" #include "CollectMacros.h" #include "Compiler.h" #include "Config.h" @@ -166,27 +167,6 @@ class CppFilePreambleCallbacks : public PreambleCallbacks { collectPragmaMarksCallback(*SourceMgr, Marks)); } - static bool isLikelyForwardingFunction(FunctionTemplateDecl *FT) { - const auto *FD = FT->getTemplatedDecl(); - const auto NumParams = FD->getNumParams(); - // Check whether its last parameter is a parameter pack... - if (NumParams > 0) { - const auto *LastParam = FD->getParamDecl(NumParams - 1); - if (const auto *PET = dyn_cast<PackExpansionType>(LastParam->getType())) { - // ... of the type T&&... or T... - const auto BaseType = PET->getPattern().getNonReferenceType(); - if (const auto *TTPT = - dyn_cast<TemplateTypeParmType>(BaseType.getTypePtr())) { - // ... whose template parameter comes from the function directly - if (FT->getTemplateParameters()->getDepth() == TTPT->getDepth()) { - return true; - } - } - } - } - return false; - } - bool shouldSkipFunctionBody(Decl *D) override { // Usually we don't need to look inside the bodies of header functions // to understand the program. However when forwarding function like >From f0d886eecf613bbf913629acd8951ee9ac8ab242 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Tue, 18 Nov 2025 14:07:14 +0100 Subject: [PATCH 02/13] clangd: Added new unittest for finding more refs to constructors Constructor calls hidden behind make_unique are currently not found, but very useful, this test expects to find them in the main index. --- .../clangd/unittests/XRefsTests.cpp | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index 7ed08d7cce3d3..51e29f5f1af43 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -13,6 +13,7 @@ #include "SyncAPI.h" #include "TestFS.h" #include "TestTU.h" +#include "TestWorkspace.h" #include "XRefs.h" #include "index/MemIndex.h" #include "clang/AST/Decl.h" @@ -2713,6 +2714,36 @@ TEST(FindReferences, NoQueryForLocalSymbols) { } } +TEST(FindReferences, ForwardingInIndex) { + Annotations Header(R"cpp( + namespace std { + template <class T> T &&forward(T &t); + template <class T, class... Args> T *make_unique(Args &&...args) { + return new T(std::forward<Args>(args)...); + } + } + struct Test { + [[T^est]](){} + }; + )cpp"); + Annotations Main(R"cpp( + #include "header.hpp" + int main() { + auto a = std::[[make_unique]]<Test>(); + } + )cpp"); + TestWorkspace TW; + TW.addMainFile("header.hpp", Header.code()); + TW.addMainFile("main.cpp", Main.code()); + auto AST = TW.openFile("header.hpp").value(); + auto Index = TW.index(); + + EXPECT_THAT(findReferences(AST, Header.point(), 0, Index.get(), + /*AddContext*/ true) + .References, + ElementsAre(rangeIs(Header.range()), rangeIs(Main.range()))); +} + TEST(GetNonLocalDeclRefs, All) { struct Case { llvm::StringRef AnnotatedCode; >From 36972c5dab08029a3fc71e7730d26c5df4333cd3 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Wed, 26 Nov 2025 23:38:11 +0100 Subject: [PATCH 03/13] clangd: Collecting constructor references through forwarding in index --- .../clangd/index/SymbolCollector.cpp | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 39c479b5f4d5b..aed74ddac1f46 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -29,6 +29,7 @@ #include "clang/AST/DeclTemplate.h" #include "clang/AST/DeclarationName.h" #include "clang/AST/Expr.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/Basic/FileEntry.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" @@ -576,6 +577,21 @@ SymbolCollector::getRefContainer(const Decl *Enclosing, return Enclosing; } +class ForwardVisitor : public RecursiveASTVisitor<ForwardVisitor> { +public: + ForwardVisitor() {} + + bool VisitCXXConstructExpr(CXXConstructExpr *E) { + if (auto *Callee = E->getConstructor()) { + Constructors.push_back(Callee); + } + return true; + } + + // Output of this visitor + std::vector<CXXConstructorDecl *> Constructors{}; +}; + // Always return true to continue indexing. bool SymbolCollector::handleDeclOccurrence( const Decl *D, index::SymbolRoleSet Roles, @@ -654,6 +670,26 @@ bool SymbolCollector::handleDeclOccurrence( // occurrence inside the base-specifier. processRelations(*ND, ID, Relations); + if (auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D)) { + if (auto *FT = FD->getDescribedFunctionTemplate(); + FT && isLikelyForwardingFunction(FT)) { + ForwardVisitor FS{}; + for (auto *Specialized : FT->specializations()) { + FS.TraverseStmt(Specialized->getBody()); + } + auto FileLoc = SM.getFileLoc(Loc); + auto FID = SM.getFileID(FileLoc); + if (Opts.RefsInHeaders || FID == SM.getMainFileID()) { + for (auto *Constructor : FS.Constructors) { + addRef(getSymbolIDCached(Constructor), + SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, + getRefContainer(ASTNode.Parent, Opts), + isSpelled(FileLoc, *ND)}); + } + } + } + } + bool CollectRef = static_cast<bool>(Opts.RefFilter & toRefKind(Roles)); // Unlike other fields, e.g. Symbols (which use spelling locations), we use // file locations for references (as it aligns the behavior of clangd's >From 52a924d3bff04c64a06252662b16bc1b51c1582e Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Wed, 26 Nov 2025 23:48:15 +0100 Subject: [PATCH 04/13] clangd: rename new visitor to better reflect task --- clang-tools-extra/clangd/index/SymbolCollector.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index aed74ddac1f46..f08e753eadda5 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -577,9 +577,10 @@ SymbolCollector::getRefContainer(const Decl *Enclosing, return Enclosing; } -class ForwardVisitor : public RecursiveASTVisitor<ForwardVisitor> { +class ForwardingToConstructorVisitor + : public RecursiveASTVisitor<ForwardingToConstructorVisitor> { public: - ForwardVisitor() {} + ForwardingToConstructorVisitor() {} bool VisitCXXConstructExpr(CXXConstructExpr *E) { if (auto *Callee = E->getConstructor()) { @@ -673,14 +674,14 @@ bool SymbolCollector::handleDeclOccurrence( if (auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D)) { if (auto *FT = FD->getDescribedFunctionTemplate(); FT && isLikelyForwardingFunction(FT)) { - ForwardVisitor FS{}; + ForwardingToConstructorVisitor Visitor{}; for (auto *Specialized : FT->specializations()) { - FS.TraverseStmt(Specialized->getBody()); + Visitor.TraverseStmt(Specialized->getBody()); } auto FileLoc = SM.getFileLoc(Loc); auto FID = SM.getFileID(FileLoc); if (Opts.RefsInHeaders || FID == SM.getMainFileID()) { - for (auto *Constructor : FS.Constructors) { + for (auto *Constructor : Visitor.Constructors) { addRef(getSymbolIDCached(Constructor), SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, getRefContainer(ASTNode.Parent, Opts), >From 8a534aa88ee1d33158c3efe37139056bf4fba0b7 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Thu, 27 Nov 2025 00:03:10 +0100 Subject: [PATCH 05/13] clangd: moving new ref collection to correct location --- .../clangd/index/SymbolCollector.cpp | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index f08e753eadda5..11afd16e5f99d 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -671,26 +671,6 @@ bool SymbolCollector::handleDeclOccurrence( // occurrence inside the base-specifier. processRelations(*ND, ID, Relations); - if (auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D)) { - if (auto *FT = FD->getDescribedFunctionTemplate(); - FT && isLikelyForwardingFunction(FT)) { - ForwardingToConstructorVisitor Visitor{}; - for (auto *Specialized : FT->specializations()) { - Visitor.TraverseStmt(Specialized->getBody()); - } - auto FileLoc = SM.getFileLoc(Loc); - auto FID = SM.getFileID(FileLoc); - if (Opts.RefsInHeaders || FID == SM.getMainFileID()) { - for (auto *Constructor : Visitor.Constructors) { - addRef(getSymbolIDCached(Constructor), - SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, - getRefContainer(ASTNode.Parent, Opts), - isSpelled(FileLoc, *ND)}); - } - } - } - } - bool CollectRef = static_cast<bool>(Opts.RefFilter & toRefKind(Roles)); // Unlike other fields, e.g. Symbols (which use spelling locations), we use // file locations for references (as it aligns the behavior of clangd's @@ -706,6 +686,27 @@ bool SymbolCollector::handleDeclOccurrence( addRef(ID, SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, getRefContainer(ASTNode.Parent, Opts), isSpelled(FileLoc, *ND)}); + // Also collect indirect constructor calls like `make_unique` + if (auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D)) { + if (auto *FT = FD->getDescribedFunctionTemplate(); + FT && isLikelyForwardingFunction(FT)) { + ForwardingToConstructorVisitor Visitor{}; + for (auto *Specialized : FT->specializations()) { + Visitor.TraverseStmt(Specialized->getBody()); + } + auto FileLoc = SM.getFileLoc(Loc); + auto FID = SM.getFileID(FileLoc); + if (Opts.RefsInHeaders || FID == SM.getMainFileID()) { + for (auto *Constructor : Visitor.Constructors) { + addRef(getSymbolIDCached(Constructor), + SymbolRef{FileLoc, FID, Roles, + index::getSymbolInfo(ND).Kind, + getRefContainer(ASTNode.Parent, Opts), + isSpelled(FileLoc, *ND)}); + } + } + } + } } } // Don't continue indexing if this is a mere reference. >From 484ba5620270ad3bb3dd14ae07997a7cf8c057aa Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Thu, 27 Nov 2025 00:13:39 +0100 Subject: [PATCH 06/13] clangd: test include formatting --- clang-tools-extra/clangd/unittests/XRefsTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index 51e29f5f1af43..d3b6faa5fa25f 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -5,8 +5,8 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===----------------------------------------------------------------------===// -#include "Annotations.h" #include "AST.h" +#include "Annotations.h" #include "ParsedAST.h" #include "Protocol.h" #include "SourceCode.h" >From 8c6928c3893abc2d023bc40fa4ca0c6616a9498a Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Thu, 27 Nov 2025 00:39:32 +0100 Subject: [PATCH 07/13] clangd: removed redundant code --- .../clangd/index/SymbolCollector.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 11afd16e5f99d..54e4333870978 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -694,16 +694,11 @@ bool SymbolCollector::handleDeclOccurrence( for (auto *Specialized : FT->specializations()) { Visitor.TraverseStmt(Specialized->getBody()); } - auto FileLoc = SM.getFileLoc(Loc); - auto FID = SM.getFileID(FileLoc); - if (Opts.RefsInHeaders || FID == SM.getMainFileID()) { - for (auto *Constructor : Visitor.Constructors) { - addRef(getSymbolIDCached(Constructor), - SymbolRef{FileLoc, FID, Roles, - index::getSymbolInfo(ND).Kind, - getRefContainer(ASTNode.Parent, Opts), - isSpelled(FileLoc, *ND)}); - } + for (auto *Constructor : Visitor.Constructors) { + addRef(getSymbolIDCached(Constructor), + SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, + getRefContainer(ASTNode.Parent, Opts), + isSpelled(FileLoc, *ND)}); } } } >From 7f4693cba8616a6870ad78bb7a85f595fc4f953c Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Fri, 28 Nov 2025 19:31:58 +0100 Subject: [PATCH 08/13] clangd: draft commit, trying different direction idea is to directly at the instantiation index the template for constructor calls. For some reason the function declaration never is an instantiation though. --- .../clangd/index/SymbolCollector.cpp | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 54e4333870978..2b5f4541d6fa7 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -688,17 +688,18 @@ bool SymbolCollector::handleDeclOccurrence( isSpelled(FileLoc, *ND)}); // Also collect indirect constructor calls like `make_unique` if (auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D)) { - if (auto *FT = FD->getDescribedFunctionTemplate(); - FT && isLikelyForwardingFunction(FT)) { - ForwardingToConstructorVisitor Visitor{}; - for (auto *Specialized : FT->specializations()) { - Visitor.TraverseStmt(Specialized->getBody()); - } - for (auto *Constructor : Visitor.Constructors) { - addRef(getSymbolIDCached(Constructor), - SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, - getRefContainer(ASTNode.Parent, Opts), - isSpelled(FileLoc, *ND)}); + if (FD->isTemplateInstantiation()) { + if (auto *PT = FD->getPrimaryTemplate(); + PT && isLikelyForwardingFunction(PT)) { + ForwardingToConstructorVisitor Visitor{}; + Visitor.TraverseStmt(FD->getBody()); + for (auto *Constructor : Visitor.Constructors) { + addRef(getSymbolIDCached(Constructor), + SymbolRef{FileLoc, FID, Roles, + index::getSymbolInfo(ND).Kind, + getRefContainer(ASTNode.Parent, Opts), + isSpelled(FileLoc, *ND)}); + } } } } >From 03e231268eaaf20c7bd6e640ec368762bb2ad407 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Sat, 29 Nov 2025 22:34:45 +0100 Subject: [PATCH 09/13] clangd: Fixed using correct declaration for indexing constructors --- .../clangd/index/SymbolCollector.cpp | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 2b5f4541d6fa7..bc5ebbef31fbd 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -687,19 +687,17 @@ bool SymbolCollector::handleDeclOccurrence( getRefContainer(ASTNode.Parent, Opts), isSpelled(FileLoc, *ND)}); // Also collect indirect constructor calls like `make_unique` - if (auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D)) { - if (FD->isTemplateInstantiation()) { - if (auto *PT = FD->getPrimaryTemplate(); - PT && isLikelyForwardingFunction(PT)) { - ForwardingToConstructorVisitor Visitor{}; - Visitor.TraverseStmt(FD->getBody()); - for (auto *Constructor : Visitor.Constructors) { - addRef(getSymbolIDCached(Constructor), - SymbolRef{FileLoc, FID, Roles, - index::getSymbolInfo(ND).Kind, - getRefContainer(ASTNode.Parent, Opts), - isSpelled(FileLoc, *ND)}); - } + if (auto *FD = llvm::dyn_cast<clang::FunctionDecl>(ASTNode.OrigD); + FD && FD->isTemplateInstantiation()) { + if (auto *PT = FD->getPrimaryTemplate(); + PT && isLikelyForwardingFunction(PT)) { + ForwardingToConstructorVisitor Visitor{}; + Visitor.TraverseStmt(FD->getBody()); + for (auto *Constructor : Visitor.Constructors) { + addRef(getSymbolIDCached(Constructor), + SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, + getRefContainer(ASTNode.Parent, Opts), + isSpelled(FileLoc, *ND)}); } } } >From 5877555de0939dc4810a9d4d4e7d84ed4a9a9a0c Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Sun, 30 Nov 2025 18:15:06 +0100 Subject: [PATCH 10/13] clangd: Added finding constructors through forwards for AST --- clang-tools-extra/clangd/XRefs.cpp | 53 ++++++++++++++++++- .../clangd/unittests/XRefsTests.cpp | 26 +++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index ef45acf501612..5ce55ec474428 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -916,6 +916,47 @@ std::vector<DocumentLink> getDocumentLinks(ParsedAST &AST) { namespace { +class ForwardingToConstructorVisitor + : public RecursiveASTVisitor<ForwardingToConstructorVisitor> { +public: + ForwardingToConstructorVisitor( + llvm::DenseSet<const CXXConstructorDecl *> &TargetConstructors) + : Targets(TargetConstructors) {} + + bool VisitCXXConstructExpr(CXXConstructExpr *E) { + if (auto *Callee = E->getConstructor()) { + if (Targets.contains(Callee)) { + ConstructorFound = true; + } + } + // It is enough to find 1 constructor + return !ConstructorFound; + } + + // Output of this visitor + bool ConstructorFound = false; + +private: + llvm::DenseSet<const CXXConstructorDecl *> &Targets; +}; + +bool forwardsToConstructor( + const Decl *D, + llvm::DenseSet<const CXXConstructorDecl *> &TargetConstructors) { + if (!TargetConstructors.empty()) { + if (auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); + FD && FD->isTemplateInstantiation()) { + if (auto *PT = FD->getPrimaryTemplate(); + PT && isLikelyForwardingFunction(PT)) { + ForwardingToConstructorVisitor Visitor{TargetConstructors}; + Visitor.TraverseStmt(FD->getBody()); + return Visitor.ConstructorFound; + } + } + } + return false; +} + /// Collects references to symbols within the main file. class ReferenceFinder : public index::IndexDataConsumer { public: @@ -933,8 +974,12 @@ class ReferenceFinder : public index::IndexDataConsumer { const llvm::ArrayRef<const NamedDecl *> Targets, bool PerToken) : PerToken(PerToken), AST(AST) { - for (const NamedDecl *ND : Targets) + for (const NamedDecl *ND : Targets) { TargetDecls.insert(ND->getCanonicalDecl()); + if (auto *Constructor = llvm::dyn_cast<clang::CXXConstructorDecl>(ND)) { + TargetConstructors.insert(Constructor); + } + } } std::vector<Reference> take() && { @@ -960,8 +1005,10 @@ class ReferenceFinder : public index::IndexDataConsumer { llvm::ArrayRef<index::SymbolRelation> Relations, SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { - if (!TargetDecls.contains(D->getCanonicalDecl())) + if (!TargetDecls.contains(D->getCanonicalDecl()) && + !forwardsToConstructor(ASTNode.OrigD, TargetConstructors)) { return true; + } const SourceManager &SM = AST.getSourceManager(); if (!isInsideMainFile(Loc, SM)) return true; @@ -1002,6 +1049,8 @@ class ReferenceFinder : public index::IndexDataConsumer { std::vector<Reference> References; const ParsedAST &AST; llvm::DenseSet<const Decl *> TargetDecls; + // Constructors need special handling since they can be hidden behind forwards + llvm::DenseSet<const CXXConstructorDecl *> TargetConstructors; }; std::vector<ReferenceFinder::Reference> diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index d3b6faa5fa25f..836c0a0134885 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -2714,6 +2714,32 @@ TEST(FindReferences, NoQueryForLocalSymbols) { } } +TEST(FindReferences, ForwardingInAST) { + Annotations Main(R"cpp( + namespace std { + template <class T> T &&forward(T &t); + template <class T, class... Args> T *make_unique(Args &&...args) { + return new T(std::forward<Args>(args)...); + } + } + + struct Test { + $Constructor[[T^est]](){} + }; + + int main() { + auto a = std::$Caller[[make_unique]]<Test>(); + } + )cpp"); + TestTU TU; + TU.Code = std::string(Main.code()); + auto AST = TU.build(); + + EXPECT_THAT(findReferences(AST, Main.point(), 0).References, + ElementsAre(rangeIs(Main.range("Constructor")), + rangeIs(Main.range("Caller")))); +} + TEST(FindReferences, ForwardingInIndex) { Annotations Header(R"cpp( namespace std { >From 949c63114e0848a36c0defd37d46337a8337a15b Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Wed, 3 Dec 2025 02:16:47 +0100 Subject: [PATCH 11/13] clangd: Added caching for forwading functions constructor calls --- clang-tools-extra/clangd/AST.h | 31 +++++++- clang-tools-extra/clangd/ParsedAST.h | 4 + clang-tools-extra/clangd/XRefs.cpp | 78 ++++++++----------- .../clangd/index/SymbolCollector.cpp | 64 ++++++++------- .../clangd/index/SymbolCollector.h | 4 + 5 files changed, 107 insertions(+), 74 deletions(-) diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index af45ae2d9022d..ffb1f269ee7ba 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -19,6 +19,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/NestedNameSpecifier.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/TypeLoc.h" #include "clang/Basic/SourceLocation.h" #include "clang/Lex/MacroInfo.h" @@ -254,9 +255,37 @@ resolveForwardingParameters(const FunctionDecl *D, unsigned MaxDepth = 10); bool isExpandedFromParameterPack(const ParmVarDecl *D); /// Heuristic that checks if FT is forwarding a parameter pack to another -/// function. (e.g. `make_unique`). +/// function (e.g. `make_unique`). bool isLikelyForwardingFunction(FunctionTemplateDecl *FT); + +class ForwardingToConstructorVisitor + : public RecursiveASTVisitor<ForwardingToConstructorVisitor> { +public: + ForwardingToConstructorVisitor() {} + + ForwardingToConstructorVisitor( + llvm::DenseSet<const CXXConstructorDecl *> *TargetConstructors) + : Targets(TargetConstructors) {} + + bool VisitCXXNewExpr(CXXNewExpr *E) { + if (auto *CE = E->getConstructExpr()) { + if (auto *Callee = CE->getConstructor()) { + if (Targets == nullptr || Targets->contains(Callee)) { + Constructors.push_back(Callee); + } + } + } + return true; + } + + // Output of this visitor + std::vector<CXXConstructorDecl *> Constructors{}; + +private: + llvm::DenseSet<const CXXConstructorDecl *> *Targets = nullptr; +}; + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h index 82fac96360488..bc9f61cb935a9 100644 --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -123,6 +123,10 @@ class ParsedAST { return Resolver.get(); } + /// Cache for constructors called through forwarding, e.g. make_unique + llvm::DenseMap<const FunctionDecl *, std::vector<CXXConstructorDecl *>> + ForwardingToConstructorCache; + private: ParsedAST(PathRef TUPath, llvm::StringRef Version, std::shared_ptr<const PreambleData> Preamble, diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 5ce55ec474428..728218203998a 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -916,47 +916,6 @@ std::vector<DocumentLink> getDocumentLinks(ParsedAST &AST) { namespace { -class ForwardingToConstructorVisitor - : public RecursiveASTVisitor<ForwardingToConstructorVisitor> { -public: - ForwardingToConstructorVisitor( - llvm::DenseSet<const CXXConstructorDecl *> &TargetConstructors) - : Targets(TargetConstructors) {} - - bool VisitCXXConstructExpr(CXXConstructExpr *E) { - if (auto *Callee = E->getConstructor()) { - if (Targets.contains(Callee)) { - ConstructorFound = true; - } - } - // It is enough to find 1 constructor - return !ConstructorFound; - } - - // Output of this visitor - bool ConstructorFound = false; - -private: - llvm::DenseSet<const CXXConstructorDecl *> &Targets; -}; - -bool forwardsToConstructor( - const Decl *D, - llvm::DenseSet<const CXXConstructorDecl *> &TargetConstructors) { - if (!TargetConstructors.empty()) { - if (auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); - FD && FD->isTemplateInstantiation()) { - if (auto *PT = FD->getPrimaryTemplate(); - PT && isLikelyForwardingFunction(PT)) { - ForwardingToConstructorVisitor Visitor{TargetConstructors}; - Visitor.TraverseStmt(FD->getBody()); - return Visitor.ConstructorFound; - } - } - } - return false; -} - /// Collects references to symbols within the main file. class ReferenceFinder : public index::IndexDataConsumer { public: @@ -970,7 +929,7 @@ class ReferenceFinder : public index::IndexDataConsumer { } }; - ReferenceFinder(const ParsedAST &AST, + ReferenceFinder(ParsedAST &AST, const llvm::ArrayRef<const NamedDecl *> Targets, bool PerToken) : PerToken(PerToken), AST(AST) { @@ -1000,13 +959,44 @@ class ReferenceFinder : public index::IndexDataConsumer { return std::move(References); } + bool forwardsToConstructor(const Decl *D) { + if (TargetConstructors.empty()) { + return false; + } + auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); + if (FD == nullptr || !FD->isTemplateInstantiation()) { + return false; + } + if (auto *PT = FD->getPrimaryTemplate(); + PT == nullptr || !isLikelyForwardingFunction(PT)) { + return false; + } + if (auto Entry = AST.ForwardingToConstructorCache.find(FD); + Entry != AST.ForwardingToConstructorCache.end()) { + for (auto *Constructor : Entry->getSecond()) { + if (TargetConstructors.contains(Constructor)) { + return true; + } + } + return false; + } + ForwardingToConstructorVisitor Visitor{&TargetConstructors}; + Visitor.TraverseStmt(FD->getBody()); + auto Iter = AST.ForwardingToConstructorCache.try_emplace( + FD, std::move(Visitor.Constructors)); + if (Iter.second) { + return !Iter.first->getSecond().empty(); + } + return false; + } + bool handleDeclOccurrence(const Decl *D, index::SymbolRoleSet Roles, llvm::ArrayRef<index::SymbolRelation> Relations, SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { if (!TargetDecls.contains(D->getCanonicalDecl()) && - !forwardsToConstructor(ASTNode.OrigD, TargetConstructors)) { + !forwardsToConstructor(ASTNode.OrigD)) { return true; } const SourceManager &SM = AST.getSourceManager(); @@ -1047,7 +1037,7 @@ class ReferenceFinder : public index::IndexDataConsumer { private: bool PerToken; // If true, report 3 references for split ObjC selector names. std::vector<Reference> References; - const ParsedAST &AST; + ParsedAST &AST; llvm::DenseSet<const Decl *> TargetDecls; // Constructors need special handling since they can be hidden behind forwards llvm::DenseSet<const CXXConstructorDecl *> TargetConstructors; diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index bc5ebbef31fbd..517932f153798 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -25,11 +25,11 @@ #include "index/SymbolLocation.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/DeclarationName.h" #include "clang/AST/Expr.h" -#include "clang/AST/RecursiveASTVisitor.h" #include "clang/Basic/FileEntry.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" @@ -52,6 +52,7 @@ #include <optional> #include <string> #include <utility> +#include <vector> namespace clang { namespace clangd { @@ -577,21 +578,29 @@ SymbolCollector::getRefContainer(const Decl *Enclosing, return Enclosing; } -class ForwardingToConstructorVisitor - : public RecursiveASTVisitor<ForwardingToConstructorVisitor> { -public: - ForwardingToConstructorVisitor() {} - - bool VisitCXXConstructExpr(CXXConstructExpr *E) { - if (auto *Callee = E->getConstructor()) { - Constructors.push_back(Callee); - } - return true; +std::vector<CXXConstructorDecl *> +SymbolCollector::findIndirectConstructors(const Decl *D) { + auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); + if (FD == nullptr || !FD->isTemplateInstantiation()) { + return {}; } - - // Output of this visitor - std::vector<CXXConstructorDecl *> Constructors{}; -}; + if (auto *PT = FD->getPrimaryTemplate(); + PT == nullptr || !isLikelyForwardingFunction(PT)) { + return {}; + } + if (auto Entry = ForwardingToConstructorCache.find(FD); + Entry != ForwardingToConstructorCache.end()) { + return Entry->getSecond(); + } + ForwardingToConstructorVisitor Visitor{}; + Visitor.TraverseStmt(FD->getBody()); + auto Iter = ForwardingToConstructorCache.try_emplace( + FD, std::move(Visitor.Constructors)); + if (Iter.second) { + return Iter.first->getSecond(); + } + return {}; +} // Always return true to continue indexing. bool SymbolCollector::handleDeclOccurrence( @@ -683,22 +692,19 @@ bool SymbolCollector::handleDeclOccurrence( auto FileLoc = SM.getFileLoc(Loc); auto FID = SM.getFileID(FileLoc); if (Opts.RefsInHeaders || FID == SM.getMainFileID()) { + auto *Container = getRefContainer(ASTNode.Parent, Opts); addRef(ID, SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, - getRefContainer(ASTNode.Parent, Opts), - isSpelled(FileLoc, *ND)}); + Container, isSpelled(FileLoc, *ND)}); // Also collect indirect constructor calls like `make_unique` - if (auto *FD = llvm::dyn_cast<clang::FunctionDecl>(ASTNode.OrigD); - FD && FD->isTemplateInstantiation()) { - if (auto *PT = FD->getPrimaryTemplate(); - PT && isLikelyForwardingFunction(PT)) { - ForwardingToConstructorVisitor Visitor{}; - Visitor.TraverseStmt(FD->getBody()); - for (auto *Constructor : Visitor.Constructors) { - addRef(getSymbolIDCached(Constructor), - SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(ND).Kind, - getRefContainer(ASTNode.Parent, Opts), - isSpelled(FileLoc, *ND)}); - } + for (auto *Constructor : findIndirectConstructors(ASTNode.OrigD)) { + if (!shouldCollectSymbol(*Constructor, *ASTCtx, Opts, IsMainFileOnly)) { + continue; + } + if (auto ConstructorID = getSymbolIDCached(Constructor)) { + addRef(ConstructorID, + SymbolRef{FileLoc, FID, Roles, + index::getSymbolInfo(Constructor).Kind, Container, + isSpelled(FileLoc, *Constructor)}); } } } diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h index e9eb27fd0f664..54e8ada0249f3 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -159,6 +159,8 @@ class SymbolCollector : public index::IndexDataConsumer { void finish() override; private: + std::vector<CXXConstructorDecl *> findIndirectConstructors(const Decl *D); + const Symbol *addDeclaration(const NamedDecl &, SymbolID, bool IsMainFileSymbol); void addDefinition(const NamedDecl &, const Symbol &DeclSymbol, @@ -230,6 +232,8 @@ class SymbolCollector : public index::IndexDataConsumer { std::unique_ptr<HeaderFileURICache> HeaderFileURIs; llvm::DenseMap<const Decl *, SymbolID> DeclToIDCache; llvm::DenseMap<const MacroInfo *, SymbolID> MacroToIDCache; + llvm::DenseMap<const FunctionDecl *, std::vector<CXXConstructorDecl *>> + ForwardingToConstructorCache; }; } // namespace clangd >From 4811317ddb6459f273008f1c6224fc5b13799087 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Wed, 3 Dec 2025 02:21:50 +0100 Subject: [PATCH 12/13] clangd: formatting --- clang-tools-extra/clangd/AST.h | 1 - 1 file changed, 1 deletion(-) diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index ffb1f269ee7ba..938689104c50a 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -258,7 +258,6 @@ bool isExpandedFromParameterPack(const ParmVarDecl *D); /// function (e.g. `make_unique`). bool isLikelyForwardingFunction(FunctionTemplateDecl *FT); - class ForwardingToConstructorVisitor : public RecursiveASTVisitor<ForwardingToConstructorVisitor> { public: >From 0aabfa235d2e48b847ab6e464838833d617a3191 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Wed, 3 Dec 2025 08:58:03 +0100 Subject: [PATCH 13/13] clangd: fixed cache not recording every constructor in AST case --- clang-tools-extra/clangd/AST.h | 11 +---------- clang-tools-extra/clangd/XRefs.cpp | 23 ++++++++++++++--------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 938689104c50a..69dec87bed0ee 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -263,16 +263,10 @@ class ForwardingToConstructorVisitor public: ForwardingToConstructorVisitor() {} - ForwardingToConstructorVisitor( - llvm::DenseSet<const CXXConstructorDecl *> *TargetConstructors) - : Targets(TargetConstructors) {} - bool VisitCXXNewExpr(CXXNewExpr *E) { if (auto *CE = E->getConstructExpr()) { if (auto *Callee = CE->getConstructor()) { - if (Targets == nullptr || Targets->contains(Callee)) { - Constructors.push_back(Callee); - } + Constructors.push_back(Callee); } } return true; @@ -280,9 +274,6 @@ class ForwardingToConstructorVisitor // Output of this visitor std::vector<CXXConstructorDecl *> Constructors{}; - -private: - llvm::DenseSet<const CXXConstructorDecl *> *Targets = nullptr; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 728218203998a..dd79c59b7b0bf 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -971,21 +971,26 @@ class ReferenceFinder : public index::IndexDataConsumer { PT == nullptr || !isLikelyForwardingFunction(PT)) { return false; } + std::vector<CXXConstructorDecl *> *Constructors = nullptr; if (auto Entry = AST.ForwardingToConstructorCache.find(FD); Entry != AST.ForwardingToConstructorCache.end()) { - for (auto *Constructor : Entry->getSecond()) { + Constructors = &Entry->getSecond(); + } + if (Constructors == nullptr) { + ForwardingToConstructorVisitor Visitor{}; + Visitor.TraverseStmt(FD->getBody()); + auto Iter = AST.ForwardingToConstructorCache.try_emplace( + FD, std::move(Visitor.Constructors)); + if (Iter.second) { + Constructors = &Iter.first->getSecond(); + } + } + if (Constructors != nullptr) { + for (auto *Constructor : *Constructors) { if (TargetConstructors.contains(Constructor)) { return true; } } - return false; - } - ForwardingToConstructorVisitor Visitor{&TargetConstructors}; - Visitor.TraverseStmt(FD->getBody()); - auto Iter = AST.ForwardingToConstructorCache.try_emplace( - FD, std::move(Visitor.Constructors)); - if (Iter.second) { - return !Iter.first->getSecond().empty(); } return false; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
