https://github.com/fitermay created https://github.com/llvm/llvm-project/pull/199480
## Summary 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. The constructor is added before the wrapper so LSP clients that auto-jump to the first target land on it; clients that present a menu still let the user reach the wrapper. This is the forward-direction counterpart to the find-references work in #169742 (clangd/clangd#716): the same `isLikelyForwardingFunction` + `searchConstructorsInForwardingFunction` machinery, applied to `locateASTReferent`. ## Changes - Extract `getForwardedConstructors(FD, cache)` in `AST.h`/`AST.cpp` 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 — no semantic change for find-references or the offline index. - 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 enclosing `CallExpr` and resolve from the actual specialization (the relation filter strips `TemplateInstantiation` candidates, so the pattern is what we receive). The forwarded ctors are added before the default `AddResultDecl(D)`. - Tests in `XRefsTests.cpp` cover: - `make_unique` - `make_shared` - Chained forwarding (three nested wrappers) - Constructor selection driven by overload resolution inside the instantiated body - Negative case: a non-forwarding template call gains no extra target ## Limitations (inherited from the find-refs infrastructure) - The visitor only follows `CXXNewExpr` and only recurses through nested calls whose callee is itself a forwarding function template. Wrappers that go through a non-template intermediate, or that construct via something other than a `new`-expression, will not be resolved. - Recursion is capped at depth 10. - Requires `PreambleParseForwardingFunctions=true` (the default) so the wrapper bodies are kept in the preamble. ## Follow-ups (not addressed here) The same helper can be wired into: - `textDocument/declaration` - `textDocument/implementation` Happy to do these in separate PRs if reviewers prefer; left out to keep this change focused on `textDocument/definition`. ## Test plan - [x] New `LocateSymbol.ConstructorForwarding*` tests pass (5 tests). - [x] Full `ClangdTests` suite passes (1388 tests; one pre-existing unrelated crash in `ExpandDeducedTypeTest.Test` excluded). - [x] `clang-format` clean on the diff. >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] [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; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
