https://github.com/fitermay updated https://github.com/llvm/llvm-project/pull/199480
>From 420d1c0d7e8ae68ec69a23d874d3e15446d93845 Mon Sep 17 00:00:00 2001 From: Yuli Fiterman <[email protected]> Date: Sun, 24 May 2026 22:13:50 -0700 Subject: [PATCH 1/2] [clangd] Navigate go-to-definition through forwarding wrappers to the constructor When the user invokes Go to Definition on a call like std::make_unique<T>(args...) or std::make_shared<T>(args...), surface the constructor of T that is actually invoked inside the wrapper alongside the wrapper itself. This is the forward-direction counterpart to the find-references work in #169742 (clangd/clangd#716): the same isLikelyForwardingFunction + searchConstructorsInForwardingFunction machinery, applied to locateASTReferent. - Extract getForwardedConstructors from the duplicated bodies of ReferenceFinder::forwardsToConstructor (XRefs.cpp) and SymbolCollector::findIndirectConstructors (index/SymbolCollector.cpp). Both existing call sites now delegate to the shared helper. - In locateASTReferent, when the targeted candidate is a function template pattern whose described template is a likely forwarding function, walk the SelectionTree to find the call expression at the cursor and resolve from the actual specialization. The constructor is added before the default AddResultDecl(D) so LSP clients that auto-jump to the first target land on it; clients that present a menu still let the user navigate to the wrapper itself. - Tests in XRefsTests.cpp cover make_unique, make_shared, chained forwarding (three levels), constructor selection driven by overload resolution inside the instantiated body, and a negative case showing that non-forwarding template calls gain no extra target. Follow-ups (not addressed here): parallel hooks for textDocument/declaration and textDocument/implementation, both of which can route through the same getForwardedConstructors helper. --- clang-tools-extra/clangd/AST.cpp | 15 ++ clang-tools-extra/clangd/AST.h | 21 +++ clang-tools-extra/clangd/XRefs.cpp | 63 +++++--- .../clangd/index/SymbolCollector.cpp | 18 +-- .../clangd/unittests/XRefsTests.cpp | 138 ++++++++++++++++++ 5 files changed, 219 insertions(+), 36 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 2ebc9b49cac55..1c5ef5c10e98f 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -1115,5 +1115,20 @@ searchConstructorsInForwardingFunction(const FunctionDecl *FD) { return Result; } +llvm::ArrayRef<const CXXConstructorDecl *> +getForwardedConstructors(const FunctionDecl *FD, + ForwardingToConstructorCache &Cache) { + if (FD == nullptr || !FD->isTemplateInstantiation()) + return {}; + if (auto It = Cache.find(FD); It != Cache.end()) + return It->getSecond(); + const auto *PT = FD->getPrimaryTemplate(); + if (PT == nullptr || !isLikelyForwardingFunction(PT)) + return {}; + auto Inserted = + Cache.try_emplace(FD, searchConstructorsInForwardingFunction(FD)); + return Inserted.first->getSecond(); +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index 2bb4943b6de0b..e55517a7e005a 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -21,6 +21,9 @@ #include "clang/AST/TypeLoc.h" #include "clang/Basic/SourceLocation.h" #include "clang/Lex/MacroInfo.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include <optional> #include <string> @@ -261,6 +264,24 @@ bool isLikelyForwardingFunction(const FunctionTemplateDecl *FT); SmallVector<const CXXConstructorDecl *, 1> searchConstructorsInForwardingFunction(const FunctionDecl *FD); +/// Cache mapping forwarding function instantiations (e.g. `make_unique<T>`) +/// to the constructors they ultimately call. +using ForwardingToConstructorCache = + llvm::DenseMap<const FunctionDecl *, + SmallVector<const CXXConstructorDecl *, 1>>; + +/// Returns the constructors that FD forwards to, if FD is a template +/// instantiation of a likely forwarding function (see +/// `isLikelyForwardingFunction`). Results are memoized in `Cache`. Returns +/// an empty range for non-forwarding functions; the cache is only populated +/// for true forwarding instantiations to keep its size bounded. +/// +/// The returned ArrayRef is owned by `Cache`; consume it before any other +/// call that may insert into the same cache. +llvm::ArrayRef<const CXXConstructorDecl *> +getForwardedConstructors(const FunctionDecl *FD, + ForwardingToConstructorCache &Cache); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 5f64f01c1ff34..6538174edbe5a 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -506,6 +506,44 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier, AddResultDecl(ID); } + // Special case: navigate through forwarding functions (e.g. + // `std::make_unique<T>(...)`) to the constructor of `T` that they call. + // The targeted forwarding function itself is still added below so users + // can fall back to it. The candidate `D` is the function template pattern + // (the relation filter strips instantiations), so we walk the + // SelectionTree to locate the call's specialization and resolve from it. + if (const auto *FD = dyn_cast<FunctionDecl>(D)) { + if (const auto *FT = FD->getDescribedFunctionTemplate(); + FT && isLikelyForwardingFunction(FT)) { + unsigned Offset = SM.getDecomposedSpellingLoc(CurLoc).second; + const FunctionTemplateDecl *TargetFT = FT->getCanonicalDecl(); + llvm::SmallPtrSet<const CXXConstructorDecl *, 1> Seen; + SelectionTree::createEach( + AST.getASTContext(), AST.getTokens(), Offset, Offset, + [&](SelectionTree ST) { + for (const auto *Cur = ST.commonAncestor(); Cur; + Cur = Cur->Parent) { + const auto *CE = Cur->ASTNode.get<CallExpr>(); + if (!CE) + continue; + const auto *Callee = CE->getDirectCallee(); + if (!Callee || !Callee->getPrimaryTemplate() || + Callee->getPrimaryTemplate()->getCanonicalDecl() != + TargetFT) + continue; + for (const auto *Ctor : getForwardedConstructors( + Callee, AST.ForwardingToConstructorCache)) + if (Seen.insert(Ctor).second) { + LocateASTReferentMetric.record(1, "forwarded-constructor"); + AddResultDecl(Ctor); + } + return true; + } + return false; + }); + } + } + LocateASTReferentMetric.record(1, "regular"); // Otherwise the target declaration is the right one. AddResultDecl(D); @@ -966,27 +1004,10 @@ class ReferenceFinder : public index::IndexDataConsumer { 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; - - SmallVector<const CXXConstructorDecl *, 1> *Constructors = nullptr; - if (auto Entry = AST.ForwardingToConstructorCache.find(FD); - Entry != AST.ForwardingToConstructorCache.end()) - Constructors = &Entry->getSecond(); - if (Constructors == nullptr) { - if (auto *PT = FD->getPrimaryTemplate(); - PT == nullptr || !isLikelyForwardingFunction(PT)) - return false; - - SmallVector<const CXXConstructorDecl *, 1> FoundConstructors = - searchConstructorsInForwardingFunction(FD); - auto Iter = AST.ForwardingToConstructorCache.try_emplace( - FD, std::move(FoundConstructors)); - Constructors = &Iter.first->getSecond(); - } - for (auto *Constructor : *Constructors) - if (TargetConstructors.contains(Constructor)) + const auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); + for (const auto *Ctor : + getForwardedConstructors(FD, AST.ForwardingToConstructorCache)) + if (TargetConstructors.contains(Ctor)) return true; return false; } diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index bd974e4c18818..4f31a68074e5a 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -578,21 +578,9 @@ SymbolCollector::getRefContainer(const Decl *Enclosing, SmallVector<const CXXConstructorDecl *, 1> SymbolCollector::findIndirectConstructors(const Decl *D) { - auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); - if (FD == nullptr || !FD->isTemplateInstantiation()) - return {}; - if (auto Entry = ForwardingToConstructorCache.find(FD); - Entry != ForwardingToConstructorCache.end()) - return Entry->getSecond(); - if (auto *PT = FD->getPrimaryTemplate(); - PT == nullptr || !isLikelyForwardingFunction(PT)) - return {}; - - SmallVector<const CXXConstructorDecl *, 1> FoundConstructors = - searchConstructorsInForwardingFunction(FD); - auto Iter = ForwardingToConstructorCache.try_emplace( - FD, std::move(FoundConstructors)); - return Iter.first->getSecond(); + const auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); + auto Forwarded = getForwardedConstructors(FD, ForwardingToConstructorCache); + return {Forwarded.begin(), Forwarded.end()}; } // Always return true to continue indexing. diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index 7f31a1e97d5d7..ef72512abb7b3 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -34,6 +34,7 @@ namespace clangd { namespace { using ::testing::AllOf; +using ::testing::Contains; using ::testing::ElementsAre; using ::testing::Eq; using ::testing::IsEmpty; @@ -2939,6 +2940,143 @@ TEST(FindReferences, TemplatedConstructorForwarding) { rangeIs(Main.range("ForwardedCaller2")))); } +TEST(LocateSymbol, ConstructorForwarding) { + Annotations Code(R"cpp( + namespace std { + template <class T> T &&forward(T &t); + template <class T, class... Args> + T *$MakeUnique[[make_unique]](Args &&...args) { + return new T(std::forward<Args>(args)...); + } + } + + struct Test { + $Ctor[[Test]]() {} + }; + + int main() { + auto a = std::ma^ke_unique<Test>(); + } + )cpp"); + TestTU TU = TestTU::withCode(Code.code()); + auto AST = TU.build(); + EXPECT_THAT( + locateSymbolAt(AST, Code.point()), + UnorderedElementsAre(sym("Test", Code.range("Ctor"), Code.range("Ctor")), + sym("make_unique", Code.range("MakeUnique"), + Code.range("MakeUnique")))); +} + +TEST(LocateSymbol, ConstructorForwardingMakeShared) { + Annotations Code(R"cpp( + namespace std { + template <class T> T &&forward(T &t); + template <class T> struct shared_ptr { + shared_ptr(T *) {} + }; + template <class T, class... Args> + shared_ptr<T> $MakeShared[[make_shared]](Args &&...args) { + return shared_ptr<T>(new T(std::forward<Args>(args)...)); + } + } + + struct Test { + $Ctor[[Test]](int) {} + }; + + int main() { + auto a = std::ma^ke_shared<Test>(1); + } + )cpp"); + TestTU TU = TestTU::withCode(Code.code()); + auto AST = TU.build(); + EXPECT_THAT( + locateSymbolAt(AST, Code.point()), + UnorderedElementsAre(sym("Test", Code.range("Ctor"), Code.range("Ctor")), + sym("make_shared", Code.range("MakeShared"), + Code.range("MakeShared")))); +} + +TEST(LocateSymbol, ConstructorForwardingChained) { + Annotations Code(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)...); + } + template <class T, class... Args> + T *$MakeUnique3[[make_unique3]](Args &&...args) { + return make_unique2<T>(forward<Args>(args)...); + } + } + + struct Test { + $Ctor[[Test]]() {} + }; + + int main() { + auto a = std::ma^ke_unique3<Test>(); + } + )cpp"); + TestTU TU = TestTU::withCode(Code.code()); + auto AST = TU.build(); + EXPECT_THAT( + locateSymbolAt(AST, Code.point()), + UnorderedElementsAre(sym("Test", Code.range("Ctor"), Code.range("Ctor")), + sym("make_unique3", Code.range("MakeUnique3"), + Code.range("MakeUnique3")))); +} + +TEST(LocateSymbol, ConstructorForwardingPicksCalledOverload) { + // Overload resolution inside the instantiated body decides which + // constructor make_unique<Test>(...) actually invokes. + Annotations Code(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 { + $IntCtor[[Test]](int) {} + Test(const char *) {} + }; + + int main() { + auto a = std::ma^ke_unique<Test>(42); + } + )cpp"); + TestTU TU = TestTU::withCode(Code.code()); + auto AST = TU.build(); + EXPECT_THAT( + locateSymbolAt(AST, Code.point()), + Contains(sym("Test", Code.range("IntCtor"), Code.range("IntCtor")))); +} + +TEST(LocateSymbol, ConstructorForwardingNotTriggeredOnUnrelatedCall) { + // A regular (non-forwarding) function call should not gain extra targets. + Annotations Code(R"cpp( + template <class T> T *$Make[[make]]() { return nullptr; } + + struct Test { + Test() {} + }; + + int main() { + auto a = ma^ke<Test>(); + } + )cpp"); + TestTU TU = TestTU::withCode(Code.code()); + auto AST = TU.build(); + EXPECT_THAT(locateSymbolAt(AST, Code.point()), + ElementsAre(sym("make", Code.range("Make"), Code.range("Make")))); +} + TEST(GetNonLocalDeclRefs, All) { struct Case { llvm::StringRef AnnotatedCode; >From 3520cad445660b4c926eccc492d1157219990a76 Mon Sep 17 00:00:00 2001 From: Yuli Fiterman <[email protected]> Date: Mon, 25 May 2026 09:58:24 -0700 Subject: [PATCH 2/2] [clangd] Style cleanup for getForwardedConstructors Follow project convention sweep: - `getForwardedConstructors` now asserts non-null FD instead of silently returning empty; the two refactored callers (ReferenceFinder::forwardsToConstructor, findIndirectConstructors) null-check the dyn_cast result before calling, matching the pattern of the neighboring `isLikelyForwardingFunction` and `searchConstructorsInForwardingFunction` (which trust their callers). - Drop `llvm::` qualifier on the helper's `ArrayRef` return type to match the unqualified `SmallVector` returned by the neighboring `searchConstructorsInForwardingFunction`. - Switch `PT == nullptr` to `!PT` to match the positive-style null tests used by the existing forwarding visitor in the same file. - Use `const auto *` for the canonical-template local in the new go-to-definition hook to match surrounding locals in `locateASTReferent`. --- clang-tools-extra/clangd/AST.cpp | 7 ++++--- clang-tools-extra/clangd/AST.h | 6 +++--- clang-tools-extra/clangd/XRefs.cpp | 4 +++- clang-tools-extra/clangd/index/SymbolCollector.cpp | 2 ++ 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 1c5ef5c10e98f..046bec5d0d5e6 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -1115,15 +1115,16 @@ searchConstructorsInForwardingFunction(const FunctionDecl *FD) { return Result; } -llvm::ArrayRef<const CXXConstructorDecl *> +ArrayRef<const CXXConstructorDecl *> getForwardedConstructors(const FunctionDecl *FD, ForwardingToConstructorCache &Cache) { - if (FD == nullptr || !FD->isTemplateInstantiation()) + assert(FD && "FD must not be null"); + if (!FD->isTemplateInstantiation()) return {}; if (auto It = Cache.find(FD); It != Cache.end()) return It->getSecond(); const auto *PT = FD->getPrimaryTemplate(); - if (PT == nullptr || !isLikelyForwardingFunction(PT)) + if (!PT || !isLikelyForwardingFunction(PT)) return {}; auto Inserted = Cache.try_emplace(FD, searchConstructorsInForwardingFunction(FD)); diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h index e55517a7e005a..ab03b117771d7 100644 --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -276,9 +276,9 @@ using ForwardingToConstructorCache = /// an empty range for non-forwarding functions; the cache is only populated /// for true forwarding instantiations to keep its size bounded. /// -/// The returned ArrayRef is owned by `Cache`; consume it before any other -/// call that may insert into the same cache. -llvm::ArrayRef<const CXXConstructorDecl *> +/// FD must not be null. The returned ArrayRef is owned by `Cache`; consume +/// it before any other call that may insert into the same cache. +ArrayRef<const CXXConstructorDecl *> getForwardedConstructors(const FunctionDecl *FD, ForwardingToConstructorCache &Cache); diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 6538174edbe5a..14cce7a5069d4 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -516,7 +516,7 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier, if (const auto *FT = FD->getDescribedFunctionTemplate(); FT && isLikelyForwardingFunction(FT)) { unsigned Offset = SM.getDecomposedSpellingLoc(CurLoc).second; - const FunctionTemplateDecl *TargetFT = FT->getCanonicalDecl(); + const auto *TargetFT = FT->getCanonicalDecl(); llvm::SmallPtrSet<const CXXConstructorDecl *, 1> Seen; SelectionTree::createEach( AST.getASTContext(), AST.getTokens(), Offset, Offset, @@ -1005,6 +1005,8 @@ class ReferenceFinder : public index::IndexDataConsumer { if (TargetConstructors.empty()) return false; const auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); + if (!FD) + return false; for (const auto *Ctor : getForwardedConstructors(FD, AST.ForwardingToConstructorCache)) if (TargetConstructors.contains(Ctor)) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 4f31a68074e5a..53fc76c594182 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -579,6 +579,8 @@ SymbolCollector::getRefContainer(const Decl *Enclosing, SmallVector<const CXXConstructorDecl *, 1> SymbolCollector::findIndirectConstructors(const Decl *D) { const auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); + if (!FD) + return {}; auto Forwarded = getForwardedConstructors(FD, ForwardingToConstructorCache); return {Forwarded.begin(), Forwarded.end()}; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
