[clang-tools-extra] [clangd] Let DefineOutline tweak handle member function templates (PR #112345)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 15 04:04:46 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangd
@llvm/pr-subscribers-clang-tools-extra
Author: Christian Kandeler (ckandeler)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/112345.diff
2 Files Affected:
- (modified) clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp (+50-19)
- (modified) clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp (+44-7)
``````````diff
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 591a8b245260ea..14d9858b702a1d 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -109,7 +109,8 @@ 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());
@@ -130,23 +131,41 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
return QualifiedFunc.takeError();
std::string TemplatePrefix;
+ auto AddToTemplatePrefixIfApplicable = [&](const Decl *D, bool append) {
+ 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
+ if (append)
+ TemplatePrefix.append(S);
+ else
+ TemplatePrefix.insert(0, S);
+ };
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, false);
}
}
-
+
+ AddToTemplatePrefixIfApplicable(FD, true);
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 +221,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 +357,7 @@ 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 +439,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;
}
@@ -443,13 +463,24 @@ class DefineOutline : public Tweak {
SameFile = true;
// Bail out if the template parameter is unnamed.
+ // FIXME: Is this really needed? It inhibits application on
+ // e.g. std::enable_if.
for (NamedDecl *P : *Params) {
if (!P->getIdentifier())
return false;
}
}
}
-
+
+ // For function templates, the same limitations as for class templates apply.
+ if (const TemplateParameterList *Params = MD->getDescribedTemplateParams()) {
+ 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()) {
@@ -459,7 +490,7 @@ class DefineOutline : public Tweak {
return false;
}
}
-
+
// Note that we don't check whether an implementation file exists or not in
// the prepare, since performing disk IO on each prepare request might be
// expensive.
@@ -485,7 +516,7 @@ 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..7e2b863733477d 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -111,13 +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.
EXPECT_UNAVAILABLE(R"cpp(
struct Foo {
@@ -237,7 +241,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 +394,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 +403,39 @@ 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);
``````````
</details>
https://github.com/llvm/llvm-project/pull/112345
More information about the cfe-commits
mailing list