https://github.com/fitermay updated https://github.com/llvm/llvm-project/pull/199480
>From c60b25b18c1106c5b544e2958280488e1256cc8e 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 with the cursor on the parens of a call to a forwarding wrapper like std::make_unique<T>(args...) or std::make_shared<T>(args...), navigate to the constructor of T that the wrapper ultimately calls. With the cursor on the wrapper's identifier, navigation continues to the wrapper itself. This mirrors the existing constructor-call behaviour: auto a = A^bc(); // navigates to the type auto a = Abc^(); // navigates to the constructor This is the forward-direction counterpart to the find-references work in #169742 (clangd/clangd#716): the same isLikelyForwardingFunction and searchConstructorsInForwardingFunction machinery, applied to locateASTReferent. - 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, with the null-check on the dyn_cast result living at the call sites (matching the neighbouring helpers, which also trust their callers). - In locateASTReferent, add an early-out before the candidate loop: if the SelectionTree's commonAncestor is a CallExpr (i.e. the cursor is on the call itself, not on the callee identifier or an argument), and the call's direct callee is a forwarding-function instantiation, AddResultDecl for the forwarded constructor and return. The candidate loop continues to handle the cursor-on- identifier case unchanged. - Tests in XRefsTests.cpp cover: - make_unique with the cursor on the parens (-> ctor only) - make_unique with the cursor on the identifier (-> wrapper only) - make_shared with the cursor on the parens - Chained forwarding (three nested wrappers) - Constructor selection driven by overload resolution inside the instantiated body - Negative case: a non-forwarding template call's parens-position still navigates to the function, not a constructor 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 | 16 ++ clang-tools-extra/clangd/AST.h | 21 +++ clang-tools-extra/clangd/XRefs.cpp | 63 ++++--- .../clangd/index/SymbolCollector.cpp | 18 +- .../clangd/unittests/XRefsTests.cpp | 159 ++++++++++++++++++ 5 files changed, 243 insertions(+), 34 deletions(-) diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 2ebc9b49cac55..046bec5d0d5e6 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -1115,5 +1115,21 @@ searchConstructorsInForwardingFunction(const FunctionDecl *FD) { return Result; } +ArrayRef<const CXXConstructorDecl *> +getForwardedConstructors(const FunctionDecl *FD, + ForwardingToConstructorCache &Cache) { + 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 || !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..ab03b117771d7 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. +/// +/// 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); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 5f64f01c1ff34..50e9d74de4a03 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -419,6 +419,44 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier, AST.getASTContext(), nameLocation(*Def, SM), MainFilePath); }; + // Special case: if the cursor lands directly on a call expression (i.e. + // its enclosing SelectionTree node is the CallExpr itself, not the callee + // identifier or an argument), and the call invokes a forwarding wrapper + // such as `std::make_unique<T>(...)`, navigate to the constructor of `T` + // that the wrapper ultimately calls. This mirrors the existing + // constructor-call behaviour: `Abc^()` jumps to the constructor while + // `A^bc()` jumps to the type. The hook does not fire when the cursor is + // on the wrapper's identifier; that path continues to navigate to the + // wrapper itself via the candidate loop below. + { + unsigned Offset = SM.getDecomposedSpellingLoc(CurLoc).second; + SelectionTree::createEach( + AST.getASTContext(), AST.getTokens(), Offset, Offset, + [&](SelectionTree ST) { + const SelectionTree::Node *N = ST.commonAncestor(); + if (!N) + return true; + const auto *CE = N->ASTNode.get<CallExpr>(); + if (!CE) + return true; + const auto *Callee = CE->getDirectCallee(); + if (!Callee) + return true; + llvm::SmallPtrSet<const CXXConstructorDecl *, 1> Seen; + for (const auto *Ctor : getForwardedConstructors( + Callee, AST.ForwardingToConstructorCache)) + if (Seen.insert(Ctor).second) { + LocateASTReferentMetric.record(1, "forwarded-constructor"); + AddResultDecl(Ctor); + } + return true; + }); + if (!Result.empty()) { + enhanceLocatedSymbolsFromIndex(Result, Index, MainFilePath); + return Result; + } + } + // Emit all symbol locations (declaration or definition) from AST. DeclRelationSet Relations = DeclRelation::TemplatePattern | DeclRelation::Alias; @@ -966,27 +1004,12 @@ 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()) + const auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); + if (!FD) 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)) + 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..53fc76c594182 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -578,21 +578,11 @@ 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()) + const auto *FD = llvm::dyn_cast<clang::FunctionDecl>(D); + if (!FD) 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(); + 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..d2cc9e3112506 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -2939,6 +2939,165 @@ TEST(FindReferences, TemplatedConstructorForwarding) { rangeIs(Main.range("ForwardedCaller2")))); } +TEST(LocateSymbol, ConstructorForwarding) { + // Cursor on the opening paren navigates to the called constructor, mirroring + // the behaviour of a direct constructor call (`Abc^()` -> ctor). + 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 { + $Ctor[[Test]]() {} + }; + + int main() { + auto a = std::make_unique<Test>^(); + } + )cpp"); + TestTU TU = TestTU::withCode(Code.code()); + auto AST = TU.build(); + EXPECT_THAT(locateSymbolAt(AST, Code.point()), + ElementsAre(sym("Test", Code.range("Ctor"), Code.range("Ctor")))); +} + +TEST(LocateSymbol, ConstructorForwardingCursorOnIdentifier) { + // Cursor on the wrapper's identifier still navigates to the wrapper itself, + // mirroring `A^bc()` -> type. The constructor target only appears when the + // cursor is on the call's parens (see ConstructorForwarding above). + 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 { + 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()), + ElementsAre(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> make_shared(Args &&...args) { + return shared_ptr<T>(new T(std::forward<Args>(args)...)); + } + } + + struct Test { + $Ctor[[Test]](int) {} + }; + + int main() { + auto a = std::make_shared<Test>^(1); + } + )cpp"); + TestTU TU = TestTU::withCode(Code.code()); + auto AST = TU.build(); + EXPECT_THAT(locateSymbolAt(AST, Code.point()), + ElementsAre(sym("Test", Code.range("Ctor"), Code.range("Ctor")))); +} + +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 *make_unique3(Args &&...args) { + return make_unique2<T>(forward<Args>(args)...); + } + } + + struct Test { + $Ctor[[Test]]() {} + }; + + int main() { + auto a = std::make_unique3<Test>^(); + } + )cpp"); + TestTU TU = TestTU::withCode(Code.code()); + auto AST = TU.build(); + EXPECT_THAT(locateSymbolAt(AST, Code.point()), + ElementsAre(sym("Test", Code.range("Ctor"), Code.range("Ctor")))); +} + +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::make_unique<Test>^(42); + } + )cpp"); + TestTU TU = TestTU::withCode(Code.code()); + auto AST = TU.build(); + EXPECT_THAT( + locateSymbolAt(AST, Code.point()), + ElementsAre(sym("Test", Code.range("IntCtor"), Code.range("IntCtor")))); +} + +TEST(LocateSymbol, ConstructorForwardingNotTriggeredOnUnrelatedCall) { + // Even with the cursor on the parens, a non-forwarding call falls through + // to the regular target: just the called function, no constructor. + Annotations Code(R"cpp( + template <class T> T *$Make[[make]]() { return nullptr; } + + struct Test { + Test() {} + }; + + int main() { + auto a = make<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
