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 1/8] 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 2/8] 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 3/8] 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 4/8] 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 5/8] 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 6/8] 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 7/8] 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 8/8] 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)}); + } } } } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
