[clang-tools-extra] [clangd] Navigate go-to-definition through forwarding wrappers to the constructor (PR #199480)
Yuli Fiterman via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 25 20:11:27 PDT 2026
https://github.com/fitermay updated https://github.com/llvm/llvm-project/pull/199480
>From d1f33c605aa84ebb12e25105aaa2428cb31d2e65 Mon Sep 17 00:00:00 2001
From: Yuli Fiterman <fiterman at google.com>
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 | 66 ++++++++++++-----
.../clangd/index/SymbolCollector.cpp | 19 +----
.../clangd/index/SymbolCollector.h | 6 +-
.../clangd/unittests/XRefsTests.cpp | 73 +++++++++++++++++++
6 files changed, 163 insertions(+), 38 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..788f0b7a0c48e 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -196,6 +196,22 @@ getDeclAtPosition(ParsedAST &AST, SourceLocation Pos, DeclRelationSet Relations,
return Result;
}
+// Returns the deepest CallExpr whose selection-tree commonAncestor is the call
+// itself at `Loc` (i.e. the cursor lands on the call's parens area, not on a
+// child like the callee identifier or an argument). Returns null otherwise.
+const CallExpr *findEnclosingCallAt(ParsedAST &AST, SourceLocation Loc) {
+ unsigned Offset = AST.getSourceManager().getDecomposedSpellingLoc(Loc).second;
+ const CallExpr *Found = nullptr;
+ SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Offset,
+ Offset, [&](SelectionTree ST) {
+ if (const SelectionTree::Node *N =
+ ST.commonAncestor())
+ Found = N->ASTNode.get<CallExpr>();
+ return true;
+ });
+ return Found;
+}
+
// Expects Loc to be a SpellingLocation, will bail out otherwise as it can't
// figure out a filename.
std::optional<Location> makeLocation(const ASTContext &AST, SourceLocation Loc,
@@ -419,6 +435,31 @@ 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.
+ if (const auto *CE = findEnclosingCallAt(AST, CurLoc)) {
+ if (const auto *Callee = CE->getDirectCallee()) {
+ 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);
+ }
+ }
+ }
+ 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 +1007,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..ce20932077c38 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -576,23 +576,12 @@ SymbolCollector::getRefContainer(const Decl *Enclosing,
return Enclosing;
}
-SmallVector<const CXXConstructorDecl *, 1>
+ArrayRef<const CXXConstructorDecl *>
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();
+ return getForwardedConstructors(FD, ForwardingToConstructorCache);
}
// Always return true to continue indexing.
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h
index 4d51d747639b1..8239aa7020c1e 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.h
+++ b/clang-tools-extra/clangd/index/SymbolCollector.h
@@ -161,9 +161,9 @@ class SymbolCollector : public index::IndexDataConsumer {
private:
// 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<const CXXConstructorDecl *, 1>
- findIndirectConstructors(const Decl *D);
+ // to those as well. The returned ArrayRef is owned by
+ // `ForwardingToConstructorCache`.
+ ArrayRef<const CXXConstructorDecl *> findIndirectConstructors(const Decl *D);
const Symbol *addDeclaration(const NamedDecl &, SymbolID,
bool IsMainFileSymbol);
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 7f31a1e97d5d7..dce033af73c1a 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2939,6 +2939,79 @@ TEST(FindReferences, TemplatedConstructorForwarding) {
rangeIs(Main.range("ForwardedCaller2"))));
}
+TEST(LocateSymbol, ConstructorForwarding) {
+ // Caret-on-paren of a forwarding wrapper navigates to the constructor it
+ // ultimately invokes; caret-on-identifier still navigates to the wrapper.
+ // This mirrors the existing direct-ctor behaviour (`Abc^()` -> ctor,
+ // `A^bc()` -> type).
+ 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)...);
+ }
+ 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)...);
+ }
+ 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)...));
+ }
+ }
+
+ // Non-forwarding template: a call to it should fall through to itself.
+ template <class T> T *$Make[[make]]() { return nullptr; }
+
+ struct Test {
+ $DefaultCtor[[Test]]() {}
+ $IntCtor[[Test]](int) {}
+ Test(const char *) {}
+ };
+
+ int main() {
+ // Caret on parens -> Test default ctor.
+ auto a = std::make_unique<Test>$ParenMU^();
+ // Caret on the wrapper identifier -> make_unique itself.
+ auto b = std::ma$IdentMU^ke_unique<Test>();
+ // make_shared works the same way; overload resolution picks Test(int).
+ auto c = std::make_shared<Test>$MakeShared^(1);
+ // Chained forwarding (three wrappers deep).
+ auto d = std::make_unique3<Test>$Chained^();
+ // Overload resolution inside the instantiated body picks Test(int).
+ auto e = std::make_unique<Test>$Overload^(42);
+ // Non-forwarding template call: no constructor target.
+ auto f = make<Test>$NonFwd^();
+ }
+ )cpp");
+ TestTU TU = TestTU::withCode(Code.code());
+ auto AST = TU.build();
+
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("ParenMU")),
+ ElementsAre(sym("Test", Code.range("DefaultCtor"),
+ Code.range("DefaultCtor"))));
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("IdentMU")),
+ ElementsAre(sym("make_unique", Code.range("MakeUnique"),
+ Code.range("MakeUnique"))));
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point("MakeShared")),
+ ElementsAre(sym("Test", Code.range("IntCtor"), Code.range("IntCtor"))));
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("Chained")),
+ ElementsAre(sym("Test", Code.range("DefaultCtor"),
+ Code.range("DefaultCtor"))));
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point("Overload")),
+ ElementsAre(sym("Test", Code.range("IntCtor"), Code.range("IntCtor"))));
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("NonFwd")),
+ ElementsAre(sym("make", Code.range("Make"), Code.range("Make"))));
+}
+
TEST(GetNonLocalDeclRefs, All) {
struct Case {
llvm::StringRef AnnotatedCode;
More information about the cfe-commits
mailing list