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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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; } >From e2d2d4ebbe601e7b0c6bee8dbaadde1854037100 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Sun, 7 Dec 2025 12:54:22 +0100 Subject: [PATCH 14/17] clangd: Find constructors through multiple forwards --- clang-tools-extra/clangd/AST.cpp | 22 ++++++++++++++ clang-tools-extra/clangd/AST.h | 11 ++----- .../clangd/unittests/XRefsTests.cpp | 29 +++++++++++++++++++ 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 4b73bdba8aaa9..0b2c6c2e721d3 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -1061,5 +1061,27 @@ bool isLikelyForwardingFunction(FunctionTemplateDecl *FT) { return false; } +bool ForwardingToConstructorVisitor::VisitCallExpr(CallExpr *E) { + if (auto *FD = E->getDirectCallee()) { + if (auto *PT = FD->getPrimaryTemplate(); + PT && isLikelyForwardingFunction(PT)) { + ForwardingToConstructorVisitor Visitor{}; + Visitor.TraverseStmt(FD->getBody()); + std::move(Visitor.Constructors.begin(), Visitor.Constructors.end(), + std::back_inserter(Constructors)); + } + } + return true; +} + +bool ForwardingToConstructorVisitor::VisitCXXNewExpr(CXXNewExpr *E) { + if (auto *CE = E->getConstructExpr()) { + if (auto *Callee = CE->getConstructor()) { + Constructors.push_back(Callee); + } + } + return true; +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 69dec87bed0ee..3a31dff458421 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -263,14 +263,9 @@ class ForwardingToConstructorVisitor public: ForwardingToConstructorVisitor() {} - bool VisitCXXNewExpr(CXXNewExpr *E) { - if (auto *CE = E->getConstructExpr()) { - if (auto *Callee = CE->getConstructor()) { - Constructors.push_back(Callee); - } - } - return true; - } + bool VisitCallExpr(CallExpr *E); + + bool VisitCXXNewExpr(CXXNewExpr *E); // Output of this visitor std::vector<CXXConstructorDecl *> Constructors{}; diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index 836c0a0134885..cdcdaa9e3a18b 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -2740,6 +2740,35 @@ TEST(FindReferences, ForwardingInAST) { rangeIs(Main.range("Caller")))); } +TEST(FindReferences, ForwardingInASTTwice) { + 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(forward<Args>(args)...); + } + template <class T, class... Args> T *make_unique2(Args &&...args) { + return make_unique<T>(forward<Args>(args)...); + } + } + + struct Test { + $Constructor[[T^est]](){} + }; + + int main() { + auto a = std::$Caller[[make_unique2]]<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 7843cf1ee374d4ad34b382bd6ee764a7c2c0ca30 Mon Sep 17 00:00:00 2001 From: Nathan Ridge <[email protected]> Date: Mon, 15 Dec 2025 02:21:03 -0500 Subject: [PATCH 15/17] Add a background index test --- .../clangd/unittests/BackgroundIndexTests.cpp | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp index ada14c9939318..6c9a3d3ee5b04 100644 --- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp @@ -1,3 +1,4 @@ +#include "Annotations.h" #include "CompileCommands.h" #include "Config.h" #include "Headers.h" @@ -233,6 +234,55 @@ TEST_F(BackgroundIndexTest, IndexTwoFiles) { fileURI("unittest:///root/B.cc")})); } +TEST_F(BackgroundIndexTest, ConstructorForwarding) { + 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 { + [[Test]](){} + }; + )cpp"); + Annotations Main(R"cpp( + #include "header.hpp" + int main() { + auto a = std::[[make_unique]]<Test>(); + } + )cpp"); + + MockFS FS; + llvm::StringMap<std::string> Storage; + size_t CacheHits = 0; + MemoryShardStorage MSS(Storage, CacheHits); + OverlayCDB CDB(/*Base=*/nullptr); + BackgroundIndex::Options Opts; + BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; }, Opts); + + FS.Files[testPath("root/header.hpp")] = Header.code(); + FS.Files[testPath("root/test.cpp")] = Main.code(); + + tooling::CompileCommand Cmd; + Cmd.Filename = testPath("root/test.cpp"); + Cmd.Directory = testPath("root"); + Cmd.CommandLine = {"clang++", testPath("root/test.cpp")}; + CDB.setCompileCommand(testPath("root/test.cpp"), Cmd); + + ASSERT_TRUE(Idx.blockUntilIdleForTest()); + + auto Syms = runFuzzyFind(Idx, "Test"); + auto Constructor = + std::find_if(Syms.begin(), Syms.end(), [](const Symbol &S) { + return S.SymInfo.Kind == index::SymbolKind::Constructor; + }); + ASSERT_TRUE(Constructor != Syms.end()); + EXPECT_THAT(getRefs(Idx, Constructor->ID), + refsAre({fileURI("unittest:///root/header.hpp"), + fileURI("unittest:///root/test.cpp")})); +} + TEST_F(BackgroundIndexTest, MainFileRefs) { MockFS FS; FS.Files[testPath("root/A.h")] = R"cpp( >From dd3327897173e49958617e3d49548ed44f71c63b Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Tue, 16 Dec 2025 10:49:09 +0100 Subject: [PATCH 16/17] clangd: delayed indexing to make function bodies available --- clang-tools-extra/clangd/AST.cpp | 3 +-- clang-tools-extra/clangd/XRefs.cpp | 25 +++++++------------ .../clangd/index/IndexAction.cpp | 3 +++ .../clangd/index/SymbolCollector.cpp | 19 ++++++-------- clang/include/clang/Index/IndexingOptions.h | 3 +++ clang/lib/Index/IndexingAction.cpp | 22 ++++++++++++++-- 6 files changed, 43 insertions(+), 32 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 0b2c6c2e721d3..736fa1dfd04af 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -1076,9 +1076,8 @@ bool ForwardingToConstructorVisitor::VisitCallExpr(CallExpr *E) { bool ForwardingToConstructorVisitor::VisitCXXNewExpr(CXXNewExpr *E) { if (auto *CE = E->getConstructExpr()) { - if (auto *Callee = CE->getConstructor()) { + if (auto *Callee = CE->getConstructor()) Constructors.push_back(Callee); - } } return true; } diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index dd79c59b7b0bf..d20651060b5c9 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -935,9 +935,8 @@ class ReferenceFinder : public index::IndexDataConsumer { : PerToken(PerToken), AST(AST) { for (const NamedDecl *ND : Targets) { TargetDecls.insert(ND->getCanonicalDecl()); - if (auto *Constructor = llvm::dyn_cast<clang::CXXConstructorDecl>(ND)) { + if (auto *Constructor = llvm::dyn_cast<clang::CXXConstructorDecl>(ND)) TargetConstructors.insert(Constructor); - } } } @@ -960,36 +959,31 @@ class ReferenceFinder : public index::IndexDataConsumer { } bool forwardsToConstructor(const Decl *D) { - if (TargetConstructors.empty()) { + if (TargetConstructors.empty()) return false; - } auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); - if (FD == nullptr || !FD->isTemplateInstantiation()) { + if (FD == nullptr || !FD->isTemplateInstantiation()) return false; - } if (auto *PT = FD->getPrimaryTemplate(); - PT == nullptr || !isLikelyForwardingFunction(PT)) { + PT == nullptr || !isLikelyForwardingFunction(PT)) return false; - } + std::vector<CXXConstructorDecl *> *Constructors = nullptr; if (auto Entry = AST.ForwardingToConstructorCache.find(FD); - Entry != AST.ForwardingToConstructorCache.end()) { + Entry != AST.ForwardingToConstructorCache.end()) 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) { + if (Iter.second) Constructors = &Iter.first->getSecond(); - } } if (Constructors != nullptr) { for (auto *Constructor : *Constructors) { - if (TargetConstructors.contains(Constructor)) { + if (TargetConstructors.contains(Constructor)) return true; - } } } return false; @@ -1001,9 +995,8 @@ class ReferenceFinder : public index::IndexDataConsumer { SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { if (!TargetDecls.contains(D->getCanonicalDecl()) && - !forwardsToConstructor(ASTNode.OrigD)) { + !forwardsToConstructor(ASTNode.OrigD)) return true; - } const SourceManager &SM = AST.getSourceManager(); if (!isInsideMainFile(Loc, SM)) return true; diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp index ed56c2a9d2e81..793516793b7d9 100644 --- a/clang-tools-extra/clangd/index/IndexAction.cpp +++ b/clang-tools-extra/clangd/index/IndexAction.cpp @@ -220,6 +220,9 @@ std::unique_ptr<FrontendAction> createStaticIndexingAction( index::IndexingOptions::SystemSymbolFilterKind::All; // We index function-local classes and its member functions only. IndexOpts.IndexFunctionLocals = true; + // We need to delay indexing so function bodies become available, this is so + // we can find constructor calls through `make_unique`. + IndexOpts.DeferIndexingToEndOfTranslationUnit = true; Opts.CollectIncludePath = true; if (Opts.Origin == SymbolOrigin::Unknown) Opts.Origin = SymbolOrigin::Static; diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 517932f153798..c1b6b059fb821 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -581,24 +581,21 @@ SymbolCollector::getRefContainer(const Decl *Enclosing, std::vector<CXXConstructorDecl *> SymbolCollector::findIndirectConstructors(const Decl *D) { auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); - if (FD == nullptr || !FD->isTemplateInstantiation()) { + if (FD == nullptr || !FD->isTemplateInstantiation()) return {}; - } if (auto *PT = FD->getPrimaryTemplate(); - PT == nullptr || !isLikelyForwardingFunction(PT)) { + PT == nullptr || !isLikelyForwardingFunction(PT)) return {}; - } if (auto Entry = ForwardingToConstructorCache.find(FD); - Entry != ForwardingToConstructorCache.end()) { + 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) { + if (Iter.second) return Iter.first->getSecond(); - } return {}; } @@ -697,15 +694,13 @@ bool SymbolCollector::handleDeclOccurrence( Container, isSpelled(FileLoc, *ND)}); // Also collect indirect constructor calls like `make_unique` for (auto *Constructor : findIndirectConstructors(ASTNode.OrigD)) { - if (!shouldCollectSymbol(*Constructor, *ASTCtx, Opts, IsMainFileOnly)) { + if (!shouldCollectSymbol(*Constructor, *ASTCtx, Opts, IsMainFileOnly)) continue; - } - if (auto ConstructorID = getSymbolIDCached(Constructor)) { + if (auto ConstructorID = getSymbolIDCached(Constructor)) addRef(ConstructorID, SymbolRef{FileLoc, FID, Roles, index::getSymbolInfo(Constructor).Kind, Container, isSpelled(FileLoc, *Constructor)}); - } } } } diff --git a/clang/include/clang/Index/IndexingOptions.h b/clang/include/clang/Index/IndexingOptions.h index 97847dd7d5d88..f184ac2ebf3f4 100644 --- a/clang/include/clang/Index/IndexingOptions.h +++ b/clang/include/clang/Index/IndexingOptions.h @@ -36,6 +36,9 @@ struct IndexingOptions { // Has no effect if IndexFunctionLocals are false. bool IndexParametersInDeclarations = false; bool IndexTemplateParameters = false; + // Some information might only be available at the end of a translation unit, + // this flag delays the indexing for this purpose. + bool DeferIndexingToEndOfTranslationUnit = false; // If set, skip indexing inside some declarations for performance. // This prevents traversal, so skipping a struct means its declaration an diff --git a/clang/lib/Index/IndexingAction.cpp b/clang/lib/Index/IndexingAction.cpp index 73a6a8c62af2e..f12e90a965d8e 100644 --- a/clang/lib/Index/IndexingAction.cpp +++ b/clang/lib/Index/IndexingAction.cpp @@ -8,6 +8,7 @@ #include "clang/Index/IndexingAction.h" #include "IndexingContext.h" +#include "clang/AST/DeclGroup.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Index/IndexDataConsumer.h" @@ -15,6 +16,7 @@ #include "clang/Lex/Preprocessor.h" #include "clang/Serialization/ASTReader.h" #include <memory> +#include <vector> using namespace clang; using namespace clang::index; @@ -101,6 +103,8 @@ class IndexASTConsumer final : public ASTConsumer { std::shared_ptr<IndexingContext> IndexCtx; std::shared_ptr<Preprocessor> PP; std::function<bool(const Decl *)> ShouldSkipFunctionBody; + bool DeferIndexingToEndOfTranslationUnit; + std::vector<DeclGroupRef> TopLevelDecls; public: IndexASTConsumer(std::shared_ptr<IndexDataConsumer> DataConsumer, @@ -110,7 +114,9 @@ class IndexASTConsumer final : public ASTConsumer { : DataConsumer(std::move(DataConsumer)), IndexCtx(new IndexingContext(Opts, *this->DataConsumer)), PP(std::move(PP)), - ShouldSkipFunctionBody(std::move(ShouldSkipFunctionBody)) { + ShouldSkipFunctionBody(std::move(ShouldSkipFunctionBody)), + DeferIndexingToEndOfTranslationUnit( + Opts.DeferIndexingToEndOfTranslationUnit) { assert(this->DataConsumer != nullptr); assert(this->PP != nullptr); } @@ -124,6 +130,10 @@ class IndexASTConsumer final : public ASTConsumer { } bool HandleTopLevelDecl(DeclGroupRef DG) override { + if (DeferIndexingToEndOfTranslationUnit) { + TopLevelDecls.emplace_back(std::move(DG)); + return true; + } return IndexCtx->indexDeclGroupRef(DG); } @@ -132,10 +142,18 @@ class IndexASTConsumer final : public ASTConsumer { } void HandleTopLevelDeclInObjCContainer(DeclGroupRef DG) override { - IndexCtx->indexDeclGroupRef(DG); + if (DeferIndexingToEndOfTranslationUnit) + TopLevelDecls.emplace_back(std::move(DG)); + else + IndexCtx->indexDeclGroupRef(DG); } void HandleTranslationUnit(ASTContext &Ctx) override { + if (DeferIndexingToEndOfTranslationUnit) { + for (auto &DG : TopLevelDecls) { + IndexCtx->indexDeclGroupRef(DG); + } + } DataConsumer->finish(); } >From 948040cb4ca69be22e0bf1b441010497efe9d40b Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Wed, 17 Dec 2025 18:59:21 +0100 Subject: [PATCH 17/17] Review comments and include cleanup --- clang-tools-extra/clangd/AST.cpp | 73 ++++++++++++++----- clang-tools-extra/clangd/AST.h | 22 ++---- clang-tools-extra/clangd/ParsedAST.h | 2 +- clang-tools-extra/clangd/XRefs.cpp | 24 +++--- .../clangd/index/IndexAction.cpp | 5 +- .../clangd/index/SymbolCollector.cpp | 29 ++++---- .../clangd/index/SymbolCollector.h | 7 +- .../clangd/unittests/BackgroundIndexTests.cpp | 1 - .../clangd/unittests/XRefsTests.cpp | 16 ++-- clang/include/clang/Index/IndexingOptions.h | 5 +- clang/lib/Index/IndexingAction.cpp | 22 ++---- 11 files changed, 116 insertions(+), 90 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 736fa1dfd04af..d770358212e23 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -18,7 +18,6 @@ #include "clang/AST/DeclTemplate.h" #include "clang/AST/DeclarationName.h" #include "clang/AST/ExprCXX.h" -#include "clang/AST/NestedNameSpecifier.h" #include "clang/AST/PrettyPrinter.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Stmt.h" @@ -32,7 +31,6 @@ #include "clang/Sema/HeuristicResolver.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" #include "llvm/Support/raw_ostream.h" @@ -1061,25 +1059,66 @@ bool isLikelyForwardingFunction(FunctionTemplateDecl *FT) { return false; } -bool ForwardingToConstructorVisitor::VisitCallExpr(CallExpr *E) { - if (auto *FD = E->getDirectCallee()) { - if (auto *PT = FD->getPrimaryTemplate(); - PT && isLikelyForwardingFunction(PT)) { - ForwardingToConstructorVisitor Visitor{}; - Visitor.TraverseStmt(FD->getBody()); - std::move(Visitor.Constructors.begin(), Visitor.Constructors.end(), - std::back_inserter(Constructors)); +class ForwardingToConstructorVisitor + : public RecursiveASTVisitor<ForwardingToConstructorVisitor> { +public: + struct SeenFunctions { + unsigned int DepthLeft; + SeenFunctions *Prev; + const FunctionDecl *Current; + }; + + ForwardingToConstructorVisitor(SeenFunctions SF, + SmallVector<CXXConstructorDecl *, 1> &Output) + : SF(std::move(SF)), Constructors(Output) {} + + bool seenFunction(const FunctionDecl *FD) { + if (SF.Current == FD) + return true; + if (SF.Prev == nullptr) + return false; + return seenFunction(SF.Prev->Current); + } + + bool VisitCallExpr(CallExpr *E) { + if (SF.DepthLeft == 0) + return true; + if (auto *FD = E->getDirectCallee()) { + // Check if we already visited this function to prevent endless recursion + if (seenFunction(FD)) + return true; + if (auto *PT = FD->getPrimaryTemplate(); + PT && isLikelyForwardingFunction(PT)) { + SeenFunctions Next{SF.DepthLeft - 1, &SF, FD}; + ForwardingToConstructorVisitor Visitor{std::move(Next), Constructors}; + Visitor.TraverseStmt(FD->getBody()); + std::move(Visitor.Constructors.begin(), Visitor.Constructors.end(), + std::back_inserter(Constructors)); + } } + return true; } - return true; -} -bool ForwardingToConstructorVisitor::VisitCXXNewExpr(CXXNewExpr *E) { - if (auto *CE = E->getConstructExpr()) { - if (auto *Callee = CE->getConstructor()) - Constructors.push_back(Callee); + bool VisitCXXNewExpr(CXXNewExpr *E) { + if (auto *CE = E->getConstructExpr()) + if (auto *Callee = CE->getConstructor()) + Constructors.push_back(Callee); + return true; } - return true; + + // List of function stack + SeenFunctions SF; + // Output of this visitor + SmallVector<CXXConstructorDecl *, 1> &Constructors; +}; + +SmallVector<CXXConstructorDecl *, 1> +searchConstructorsInForwardingFunction(const FunctionDecl *FD) { + SmallVector<CXXConstructorDecl *, 1> Result; + ForwardingToConstructorVisitor::SeenFunctions SF{10, nullptr, FD}; + ForwardingToConstructorVisitor Visitor{std::move(SF), Result}; + Visitor.TraverseStmt(FD->getBody()); + return Result; } } // namespace clangd diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 3a31dff458421..152abdffcbb84 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -18,8 +18,6 @@ #include "index/SymbolID.h" #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,22 +252,14 @@ 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`). +/// Heuristic that checks if FT is likely to be forwarding a parameter pack to +/// another function (e.g. `make_unique`). bool isLikelyForwardingFunction(FunctionTemplateDecl *FT); -class ForwardingToConstructorVisitor - : public RecursiveASTVisitor<ForwardingToConstructorVisitor> { -public: - ForwardingToConstructorVisitor() {} - - bool VisitCallExpr(CallExpr *E); - - bool VisitCXXNewExpr(CXXNewExpr *E); - - // Output of this visitor - std::vector<CXXConstructorDecl *> Constructors{}; -}; +/// Only call if FD is a likely forwarding function. Returns +/// constructors that might be forwraded to +SmallVector<CXXConstructorDecl *, 1> +searchConstructorsInForwardingFunction(const FunctionDecl *FD); } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h index bc9f61cb935a9..715909e9654cc 100644 --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -124,7 +124,7 @@ class ParsedAST { } /// Cache for constructors called through forwarding, e.g. make_unique - llvm::DenseMap<const FunctionDecl *, std::vector<CXXConstructorDecl *>> + llvm::DenseMap<const FunctionDecl *, SmallVector<CXXConstructorDecl *, 1>> ForwardingToConstructorCache; private: diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index d20651060b5c9..05a97a6af4ec1 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -16,7 +16,6 @@ #include "Quality.h" #include "Selection.h" #include "SourceCode.h" -#include "URI.h" #include "clang-include-cleaner/Analysis.h" #include "clang-include-cleaner/Types.h" #include "index/Index.h" @@ -43,7 +42,6 @@ #include "clang/AST/StmtVisitor.h" #include "clang/AST/Type.h" #include "clang/Basic/LLVM.h" -#include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" @@ -60,7 +58,6 @@ #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" -#include "llvm/ADT/SmallSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" @@ -964,28 +961,27 @@ class ReferenceFinder : public index::IndexDataConsumer { 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; - std::vector<CXXConstructorDecl *> *Constructors = nullptr; + SmallVector<CXXConstructorDecl *, 1> *Constructors = nullptr; if (auto Entry = AST.ForwardingToConstructorCache.find(FD); Entry != AST.ForwardingToConstructorCache.end()) Constructors = &Entry->getSecond(); if (Constructors == nullptr) { - ForwardingToConstructorVisitor Visitor{}; - Visitor.TraverseStmt(FD->getBody()); + if (auto *PT = FD->getPrimaryTemplate(); + PT == nullptr || !isLikelyForwardingFunction(PT)) + return false; + + SmallVector<CXXConstructorDecl *, 1> FoundConstructors = + searchConstructorsInForwardingFunction(FD); auto Iter = AST.ForwardingToConstructorCache.try_emplace( - FD, std::move(Visitor.Constructors)); + FD, std::move(FoundConstructors)); if (Iter.second) Constructors = &Iter.first->getSecond(); } - if (Constructors != nullptr) { - for (auto *Constructor : *Constructors) { + if (Constructors != nullptr) + for (auto *Constructor : *Constructors) if (TargetConstructors.contains(Constructor)) return true; - } - } return false; } diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp index 793516793b7d9..4f70e9fb88c8f 100644 --- a/clang-tools-extra/clangd/index/IndexAction.cpp +++ b/clang-tools-extra/clangd/index/IndexAction.cpp @@ -21,7 +21,6 @@ #include "clang/Frontend/FrontendAction.h" #include "clang/Index/IndexingAction.h" #include "clang/Index/IndexingOptions.h" -#include <cstddef> #include <functional> #include <memory> #include <optional> @@ -220,8 +219,8 @@ std::unique_ptr<FrontendAction> createStaticIndexingAction( index::IndexingOptions::SystemSymbolFilterKind::All; // We index function-local classes and its member functions only. IndexOpts.IndexFunctionLocals = true; - // We need to delay indexing so function bodies become available, this is so - // we can find constructor calls through `make_unique`. + // We need to delay indexing so instantiations of function bodies become + // available, this is so we can find constructor calls through `make_unique`. IndexOpts.DeferIndexingToEndOfTranslationUnit = true; Opts.CollectIncludePath = true; if (Opts.Origin == SymbolOrigin::Unknown) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index c1b6b059fb821..b55b340fa3c39 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -45,14 +45,12 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" #include "llvm/Support/ErrorHandling.h" -#include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include <cassert> #include <memory> #include <optional> #include <string> #include <utility> -#include <vector> namespace clang { namespace clangd { @@ -578,22 +576,22 @@ SymbolCollector::getRefContainer(const Decl *Enclosing, return Enclosing; } -std::vector<CXXConstructorDecl *> +SmallVector<CXXConstructorDecl *, 1> SymbolCollector::findIndirectConstructors(const Decl *D) { auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); if (FD == nullptr || !FD->isTemplateInstantiation()) return {}; - if (auto *PT = FD->getPrimaryTemplate(); - PT == nullptr || !isLikelyForwardingFunction(PT)) - return {}; if (auto Entry = ForwardingToConstructorCache.find(FD); Entry != ForwardingToConstructorCache.end()) return Entry->getSecond(); + if (auto *PT = FD->getPrimaryTemplate(); + PT == nullptr || !isLikelyForwardingFunction(PT)) + return {}; - ForwardingToConstructorVisitor Visitor{}; - Visitor.TraverseStmt(FD->getBody()); + SmallVector<CXXConstructorDecl *, 1> FoundConstructors = + searchConstructorsInForwardingFunction(FD); auto Iter = ForwardingToConstructorCache.try_emplace( - FD, std::move(Visitor.Constructors)); + FD, std::move(FoundConstructors)); if (Iter.second) return Iter.first->getSecond(); return {}; @@ -662,10 +660,12 @@ bool SymbolCollector::handleDeclOccurrence( // ND is the canonical (i.e. first) declaration. If it's in the main file // (which is not a header), then no public declaration was visible, so assume // it's main-file only. - bool IsMainFileOnly = - SM.isWrittenInMainFile(SM.getExpansionLoc(ND->getBeginLoc())) && - !isHeaderFile(SM.getFileEntryRefForID(SM.getMainFileID())->getName(), - ASTCtx->getLangOpts()); + auto CheckIsMainFileOnly = [&](const NamedDecl *Decl) { + return SM.isWrittenInMainFile(SM.getExpansionLoc(Decl->getBeginLoc())) && + !isHeaderFile(SM.getFileEntryRefForID(SM.getMainFileID())->getName(), + ASTCtx->getLangOpts()); + }; + bool IsMainFileOnly = CheckIsMainFileOnly(ND); // In C, printf is a redecl of an implicit builtin! So check OrigD instead. if (ASTNode.OrigD->isImplicit() || !shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly)) @@ -694,7 +694,8 @@ bool SymbolCollector::handleDeclOccurrence( Container, isSpelled(FileLoc, *ND)}); // Also collect indirect constructor calls like `make_unique` for (auto *Constructor : findIndirectConstructors(ASTNode.OrigD)) { - if (!shouldCollectSymbol(*Constructor, *ASTCtx, Opts, IsMainFileOnly)) + if (!shouldCollectSymbol(*Constructor, *ASTCtx, Opts, + CheckIsMainFileOnly(Constructor))) continue; if (auto ConstructorID = getSymbolIDCached(Constructor)) addRef(ConstructorID, diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h index 54e8ada0249f3..66a9b41eaa875 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -159,7 +159,10 @@ class SymbolCollector : public index::IndexDataConsumer { void finish() override; private: - std::vector<CXXConstructorDecl *> findIndirectConstructors(const Decl *D); + // If D is an instantiation of a likely forwarding function, return the + // constructors it invokes so that we can record indirect references + // to those as well. + SmallVector<CXXConstructorDecl *, 1> findIndirectConstructors(const Decl *D); const Symbol *addDeclaration(const NamedDecl &, SymbolID, bool IsMainFileSymbol); @@ -232,7 +235,7 @@ 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 *>> + llvm::DenseMap<const FunctionDecl *, SmallVector<CXXConstructorDecl *, 1>> ForwardingToConstructorCache; }; diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp index 6c9a3d3ee5b04..afd56428fca62 100644 --- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp @@ -15,7 +15,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include <deque> -#include <thread> using ::testing::_; using ::testing::AllOf; diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index cdcdaa9e3a18b..792252cba2b7e 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -312,6 +312,7 @@ MATCHER_P3(sym, Name, Decl, DefOrNone, "") { MATCHER_P(sym, Name, "") { return arg.Name == Name; } MATCHER_P(rangeIs, R, "") { return arg.Loc.range == R; } +MATCHER_P(fileIs, F, "") { return arg.Loc.uri.file() == F; } MATCHER_P(containerIs, C, "") { return arg.Loc.containerName.value_or("") == C; } @@ -2740,7 +2741,7 @@ TEST(FindReferences, ForwardingInAST) { rangeIs(Main.range("Caller")))); } -TEST(FindReferences, ForwardingInASTTwice) { +TEST(FindReferences, ForwardingInASTChained) { Annotations Main(R"cpp( namespace std { template <class T> T &&forward(T &t); @@ -2788,15 +2789,18 @@ TEST(FindReferences, ForwardingInIndex) { } )cpp"); TestWorkspace TW; - TW.addMainFile("header.hpp", Header.code()); + TW.addSource("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()))); + EXPECT_THAT( + findReferences(AST, Header.point(), 0, Index.get(), + /*AddContext*/ true) + .References, + ElementsAre( + AllOf(rangeIs(Header.range()), fileIs(testPath("header.hpp"))), + AllOf(rangeIs(Main.range()), fileIs(testPath("main.cpp"))))); } TEST(GetNonLocalDeclRefs, All) { diff --git a/clang/include/clang/Index/IndexingOptions.h b/clang/include/clang/Index/IndexingOptions.h index f184ac2ebf3f4..c670797e9fa60 100644 --- a/clang/include/clang/Index/IndexingOptions.h +++ b/clang/include/clang/Index/IndexingOptions.h @@ -37,7 +37,10 @@ struct IndexingOptions { bool IndexParametersInDeclarations = false; bool IndexTemplateParameters = false; // Some information might only be available at the end of a translation unit, - // this flag delays the indexing for this purpose. + // this flag delays the indexing for this purpose (e.g. instantiation of + // function definitions). This option only takes effect on operations that + // actually build the AST, e.g. `createIndexingAction()` and + // `createIndexingASTConsumer()`. bool DeferIndexingToEndOfTranslationUnit = false; // If set, skip indexing inside some declarations for performance. diff --git a/clang/lib/Index/IndexingAction.cpp b/clang/lib/Index/IndexingAction.cpp index f12e90a965d8e..8118ceda9cd23 100644 --- a/clang/lib/Index/IndexingAction.cpp +++ b/clang/lib/Index/IndexingAction.cpp @@ -16,7 +16,6 @@ #include "clang/Lex/Preprocessor.h" #include "clang/Serialization/ASTReader.h" #include <memory> -#include <vector> using namespace clang; using namespace clang::index; @@ -104,7 +103,6 @@ class IndexASTConsumer final : public ASTConsumer { std::shared_ptr<Preprocessor> PP; std::function<bool(const Decl *)> ShouldSkipFunctionBody; bool DeferIndexingToEndOfTranslationUnit; - std::vector<DeclGroupRef> TopLevelDecls; public: IndexASTConsumer(std::shared_ptr<IndexDataConsumer> DataConsumer, @@ -130,11 +128,9 @@ class IndexASTConsumer final : public ASTConsumer { } bool HandleTopLevelDecl(DeclGroupRef DG) override { - if (DeferIndexingToEndOfTranslationUnit) { - TopLevelDecls.emplace_back(std::move(DG)); - return true; - } - return IndexCtx->indexDeclGroupRef(DG); + if (!DeferIndexingToEndOfTranslationUnit) + return IndexCtx->indexDeclGroupRef(DG); + return true; } void HandleInterestingDecl(DeclGroupRef DG) override { @@ -142,18 +138,14 @@ class IndexASTConsumer final : public ASTConsumer { } void HandleTopLevelDeclInObjCContainer(DeclGroupRef DG) override { - if (DeferIndexingToEndOfTranslationUnit) - TopLevelDecls.emplace_back(std::move(DG)); - else + if (!DeferIndexingToEndOfTranslationUnit) IndexCtx->indexDeclGroupRef(DG); } void HandleTranslationUnit(ASTContext &Ctx) override { - if (DeferIndexingToEndOfTranslationUnit) { - for (auto &DG : TopLevelDecls) { - IndexCtx->indexDeclGroupRef(DG); - } - } + if (DeferIndexingToEndOfTranslationUnit) + for (auto *DG : Ctx.getTranslationUnitDecl()->decls()) + IndexCtx->indexTopLevelDecl(DG); DataConsumer->finish(); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
