[clang-tools-extra] [clangd] Let DefineOutline tweak handle member function templates (PR #112345)
Christian Kandeler via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 18 04:10:23 PST 2024
https://github.com/ckandeler updated https://github.com/llvm/llvm-project/pull/112345
>From 5b563ceec2c3462a00101526fa416a835a59e7c0 Mon Sep 17 00:00:00 2001
From: Christian Kandeler <christian.kandeler at qt.io>
Date: Mon, 14 Oct 2024 15:37:01 +0200
Subject: [PATCH] [clangd] Let DefineOutline tweak handle member function
templates
---
.../clangd/refactor/tweaks/DefineOutline.cpp | 67 +++++++++++++------
.../unittests/tweaks/DefineOutlineTests.cpp | 49 ++++++++++++--
2 files changed, 91 insertions(+), 25 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 591a8b245260ea..2a96b305bd7a25 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -109,14 +109,13 @@ findContextForNS(llvm::StringRef TargetNS, const DeclContext *CurContext) {
// afterwards it can be shared with define-inline code action.
llvm::Expected<std::string>
getFunctionSourceAfterReplacements(const FunctionDecl *FD,
- const tooling::Replacements &Replacements) {
+ const tooling::Replacements &Replacements,
+ bool TargetFileIsHeader) {
const auto &SM = FD->getASTContext().getSourceManager();
auto OrigFuncRange = toHalfOpenFileRange(
SM, FD->getASTContext().getLangOpts(), FD->getSourceRange());
if (!OrigFuncRange)
return error("Couldn't get range for function.");
- assert(!FD->getDescribedFunctionTemplate() &&
- "Define out-of-line doesn't apply to function templates.");
// Get new begin and end positions for the qualified function definition.
unsigned FuncBegin = SM.getFileOffset(OrigFuncRange->getBegin());
@@ -129,24 +128,38 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
if (!QualifiedFunc)
return QualifiedFunc.takeError();
+ auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
std::string TemplatePrefix;
+ auto AddToTemplatePrefixIfApplicable = [&](const Decl *D) {
+ const TemplateParameterList *Params = D->getDescribedTemplateParams();
+ if (!Params)
+ return;
+ for (Decl *P : *Params) {
+ if (auto *TTP = dyn_cast<TemplateTypeParmDecl>(P))
+ TTP->removeDefaultArgument();
+ else if (auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(P))
+ NTTP->removeDefaultArgument();
+ else if (auto *TTPD = dyn_cast<TemplateTemplateParmDecl>(P))
+ TTPD->removeDefaultArgument();
+ }
+ std::string S;
+ llvm::raw_string_ostream Stream(S);
+ Params->print(Stream, FD->getASTContext());
+ if (!S.empty())
+ *S.rbegin() = '\n'; // Replace space with newline
+ TemplatePrefix.insert(0, S);
+ };
+ AddToTemplatePrefixIfApplicable(FD);
if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(FD)) {
for (const CXXRecordDecl *Parent = MD->getParent(); Parent;
Parent =
llvm::dyn_cast_or_null<const CXXRecordDecl>(Parent->getParent())) {
- if (const TemplateParameterList *Params =
- Parent->getDescribedTemplateParams()) {
- std::string S;
- llvm::raw_string_ostream Stream(S);
- Params->print(Stream, FD->getASTContext());
- if (!S.empty())
- *S.rbegin() = '\n'; // Replace space with newline
- TemplatePrefix.insert(0, S);
- }
+ AddToTemplatePrefixIfApplicable(Parent);
}
}
- auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
+ if (TargetFileIsHeader)
+ Source.insert(0, "inline ");
if (!TemplatePrefix.empty())
Source.insert(0, TemplatePrefix);
return Source;
@@ -202,7 +215,8 @@ deleteTokensWithKind(const syntax::TokenBuffer &TokBuf, tok::TokenKind Kind,
llvm::Expected<std::string>
getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
const syntax::TokenBuffer &TokBuf,
- const HeuristicResolver *Resolver) {
+ const HeuristicResolver *Resolver,
+ bool TargetFileIsHeader) {
auto &AST = FD->getASTContext();
auto &SM = AST.getSourceManager();
@@ -337,7 +351,8 @@ getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
if (Errors)
return std::move(Errors);
- return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);
+ return getFunctionSourceAfterReplacements(FD, DeclarationCleanups,
+ TargetFileIsHeader);
}
struct InsertionPoint {
@@ -419,15 +434,15 @@ class DefineOutline : public Tweak {
Source->isOutOfLine())
return false;
- // Bail out if this is a function template or specialization, as their
+ // Bail out if this is a function template specialization, as their
// definitions need to be visible in all including translation units.
- if (Source->getDescribedFunctionTemplate())
- return false;
if (Source->getTemplateSpecializationInfo())
return false;
auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source);
if (!MD) {
+ if (Source->getDescribedFunctionTemplate())
+ return false;
// Can't outline free-standing functions in the same file.
return !SameFile;
}
@@ -450,6 +465,19 @@ class DefineOutline : public Tweak {
}
}
+ // For function templates, the same limitations as for class templates
+ // apply.
+ if (const TemplateParameterList *Params =
+ MD->getDescribedTemplateParams()) {
+ // FIXME: Is this really needed? It inhibits application on
+ // e.g. std::enable_if.
+ for (NamedDecl *P : *Params) {
+ if (!P->getIdentifier())
+ return false;
+ }
+ SameFile = true;
+ }
+
// The refactoring is meaningless for unnamed classes and namespaces,
// unless we're outlining in the same file
for (const DeclContext *DC = MD->getParent(); DC; DC = DC->getParent()) {
@@ -485,7 +513,8 @@ class DefineOutline : public Tweak {
auto FuncDef = getFunctionSourceCode(
Source, InsertionPoint->EnclosingNamespace, Sel.AST->getTokens(),
- Sel.AST->getHeuristicResolver());
+ Sel.AST->getHeuristicResolver(),
+ SameFile && isHeaderFile(Sel.AST->tuPath(), Sel.AST->getLangOpts()));
if (!FuncDef)
return FuncDef.takeError();
diff --git a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
index 6a9e90c3bfa70f..1af1bc31bf6486 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -111,11 +111,17 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
template <typename> struct Foo { void fo^o(){} };
)cpp");
- // Not available on function templates and specializations, as definition must
- // be visible to all translation units.
+ // Not available on function template specializations and free function
+ // templates.
EXPECT_UNAVAILABLE(R"cpp(
- template <typename> void fo^o() {};
- template <> void fo^o<int>() {};
+ template <typename T> void fo^o() {}
+ template <> void fo^o<int>() {}
+ )cpp");
+
+ // Not available on member function templates with unnamed template
+ // parameters.
+ EXPECT_UNAVAILABLE(R"cpp(
+ struct Foo { template <typename> void ba^r() {} };
)cpp");
// Not available on methods of unnamed classes.
@@ -237,7 +243,7 @@ TEST_F(DefineOutlineTest, ApplyTest) {
Foo(T z) __attribute__((weak)) ;
int bar;
};template <typename T>
-Foo<T>::Foo(T z) __attribute__((weak)) : bar(2){}
+inline Foo<T>::Foo(T z) __attribute__((weak)) : bar(2){}
)cpp",
""},
// Virt specifiers.
@@ -390,7 +396,7 @@ Foo<T>::Foo(T z) __attribute__((weak)) : bar(2){}
};
};template <typename T, typename ...U>
template <class V, int A>
-typename O1<T, U...>::template O2<V, A>::E O1<T, U...>::template O2<V, A>::I::foo(T, U..., V, E) { return E1; }
+inline typename O1<T, U...>::template O2<V, A>::E O1<T, U...>::template O2<V, A>::I::foo(T, U..., V, E) { return E1; }
)cpp",
""},
// Destructors
@@ -399,6 +405,37 @@ typename O1<T, U...>::template O2<V, A>::E O1<T, U...>::template O2<V, A>::I::fo
"class A { ~A(); };",
"A::~A(){} ",
},
+
+ // Member template
+ {
+ R"cpp(
+ struct Foo {
+ template <typename T, bool B = true>
+ void ^bar() {}
+ };)cpp",
+ R"cpp(
+ struct Foo {
+ template <typename T, bool B = true>
+ void bar() ;
+ };template <typename T, bool B>
+inline void Foo::bar() {}
+)cpp",
+ ""},
+
+ // Class template with member template
+ {
+ R"cpp(
+ template <typename T> struct Foo {
+ template <typename U> void ^bar(const T& t, const U& u) {}
+ };)cpp",
+ R"cpp(
+ template <typename T> struct Foo {
+ template <typename U> void bar(const T& t, const U& u) ;
+ };template <typename T>
+template <typename U>
+inline void Foo<T>::bar(const T& t, const U& u) {}
+)cpp",
+ ""},
};
for (const auto &Case : Cases) {
SCOPED_TRACE(Case.Test);
More information about the cfe-commits
mailing list