Skip to content

Commit 8a6a76b

Browse files
ckandelerckandeler
authored
[clangd] Let DefineOutline tweak handle member function templates (llvm#112345)
1 parent cac6f21 commit 8a6a76b

File tree

2 files changed

+91
-25
lines changed

2 files changed

+91
-25
lines changed

‎clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp‎

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,13 @@ findContextForNS(llvm::StringRef TargetNS, const DeclContext *CurContext) {
109109
// afterwards it can be shared with define-inline code action.
110110
llvm::Expected<std::string>
111111
getFunctionSourceAfterReplacements(const FunctionDecl *FD,
112-
const tooling::Replacements &Replacements) {
112+
const tooling::Replacements &Replacements,
113+
bool TargetFileIsHeader) {
113114
const auto &SM = FD->getASTContext().getSourceManager();
114115
auto OrigFuncRange = toHalfOpenFileRange(
115116
SM, FD->getASTContext().getLangOpts(), FD->getSourceRange());
116117
if (!OrigFuncRange)
117118
return error("Couldn't get range for function.");
118-
assert(!FD->getDescribedFunctionTemplate() &&
119-
"Define out-of-line doesn't apply to function templates.");
120119

121120
// Get new begin and end positions for the qualified function definition.
122121
unsigned FuncBegin = SM.getFileOffset(OrigFuncRange->getBegin());
@@ -129,24 +128,38 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
129128
if (!QualifiedFunc)
130129
return QualifiedFunc.takeError();
131130

131+
auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
132132
std::string TemplatePrefix;
133+
auto AddToTemplatePrefixIfApplicable = [&](const Decl *D) {
134+
const TemplateParameterList *Params = D->getDescribedTemplateParams();
135+
if (!Params)
136+
return;
137+
for (Decl *P : *Params) {
138+
if (auto *TTP = dyn_cast<TemplateTypeParmDecl>(P))
139+
TTP->removeDefaultArgument();
140+
else if (auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(P))
141+
NTTP->removeDefaultArgument();
142+
else if (auto *TTPD = dyn_cast<TemplateTemplateParmDecl>(P))
143+
TTPD->removeDefaultArgument();
144+
}
145+
std::string S;
146+
llvm::raw_string_ostream Stream(S);
147+
Params->print(Stream, FD->getASTContext());
148+
if (!S.empty())
149+
*S.rbegin() = '\n'; // Replace space with newline
150+
TemplatePrefix.insert(0, S);
151+
};
152+
AddToTemplatePrefixIfApplicable(FD);
133153
if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(FD)) {
134154
for (const CXXRecordDecl *Parent = MD->getParent(); Parent;
135155
Parent =
136156
llvm::dyn_cast_or_null<const CXXRecordDecl>(Parent->getParent())) {
137-
if (const TemplateParameterList *Params =
138-
Parent->getDescribedTemplateParams()) {
139-
std::string S;
140-
llvm::raw_string_ostream Stream(S);
141-
Params->print(Stream, FD->getASTContext());
142-
if (!S.empty())
143-
*S.rbegin() = '\n'; // Replace space with newline
144-
TemplatePrefix.insert(0, S);
145-
}
157+
AddToTemplatePrefixIfApplicable(Parent);
146158
}
147159
}
148160

149-
auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
161+
if (TargetFileIsHeader)
162+
Source.insert(0, "inline ");
150163
if (!TemplatePrefix.empty())
151164
Source.insert(0, TemplatePrefix);
152165
return Source;
@@ -202,7 +215,8 @@ deleteTokensWithKind(const syntax::TokenBuffer &TokBuf, tok::TokenKind Kind,
202215
llvm::Expected<std::string>
203216
getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
204217
const syntax::TokenBuffer &TokBuf,
205-
const HeuristicResolver *Resolver) {
218+
const HeuristicResolver *Resolver,
219+
bool TargetFileIsHeader) {
206220
auto &AST = FD->getASTContext();
207221
auto &SM = AST.getSourceManager();
208222

@@ -337,7 +351,8 @@ getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
337351

338352
if (Errors)
339353
return std::move(Errors);
340-
return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);
354+
return getFunctionSourceAfterReplacements(FD, DeclarationCleanups,
355+
TargetFileIsHeader);
341356
}
342357

343358
struct InsertionPoint {
@@ -419,15 +434,15 @@ class DefineOutline : public Tweak {
419434
Source->isOutOfLine())
420435
return false;
421436

422-
// Bail out if this is a function template or specialization, as their
437+
// Bail out if this is a function template specialization, as their
423438
// definitions need to be visible in all including translation units.
424-
if (Source->getDescribedFunctionTemplate())
425-
return false;
426439
if (Source->getTemplateSpecializationInfo())
427440
return false;
428441

429442
auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source);
430443
if (!MD) {
444+
if (Source->getDescribedFunctionTemplate())
445+
return false;
431446
// Can't outline free-standing functions in the same file.
432447
return !SameFile;
433448
}
@@ -450,6 +465,19 @@ class DefineOutline : public Tweak {
450465
}
451466
}
452467

468+
// For function templates, the same limitations as for class templates
469+
// apply.
470+
if (const TemplateParameterList *Params =
471+
MD->getDescribedTemplateParams()) {
472+
// FIXME: Is this really needed? It inhibits application on
473+
// e.g. std::enable_if.
474+
for (NamedDecl *P : *Params) {
475+
if (!P->getIdentifier())
476+
return false;
477+
}
478+
SameFile = true;
479+
}
480+
453481
// The refactoring is meaningless for unnamed classes and namespaces,
454482
// unless we're outlining in the same file
455483
for (const DeclContext *DC = MD->getParent(); DC; DC = DC->getParent()) {
@@ -485,7 +513,8 @@ class DefineOutline : public Tweak {
485513

486514
auto FuncDef = getFunctionSourceCode(
487515
Source, InsertionPoint->EnclosingNamespace, Sel.AST->getTokens(),
488-
Sel.AST->getHeuristicResolver());
516+
Sel.AST->getHeuristicResolver(),
517+
SameFile && isHeaderFile(Sel.AST->tuPath(), Sel.AST->getLangOpts()));
489518
if (!FuncDef)
490519
return FuncDef.takeError();
491520

‎clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,17 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
111111
template <typename> struct Foo { void fo^o(){} };
112112
)cpp");
113113

114-
// Not available on function templates and specializations, as definition must
115-
// be visible to all translation units.
114+
// Not available on function template specializations and free function
115+
// templates.
116116
EXPECT_UNAVAILABLE(R"cpp(
117-
template <typename> void fo^o() {};
118-
template <> void fo^o<int>() {};
117+
template <typename T> void fo^o() {}
118+
template <> void fo^o<int>() {}
119+
)cpp");
120+
121+
// Not available on member function templates with unnamed template
122+
// parameters.
123+
EXPECT_UNAVAILABLE(R"cpp(
124+
struct Foo { template <typename> void ba^r() {} };
119125
)cpp");
120126

121127
// Not available on methods of unnamed classes.
@@ -237,7 +243,7 @@ TEST_F(DefineOutlineTest, ApplyTest) {
237243
Foo(T z) __attribute__((weak)) ;
238244
int bar;
239245
};template <typename T>
240-
Foo<T>::Foo(T z) __attribute__((weak)) : bar(2){}
246+
inline Foo<T>::Foo(T z) __attribute__((weak)) : bar(2){}
241247
)cpp",
242248
""},
243249
// Virt specifiers.
@@ -390,7 +396,7 @@ Foo<T>::Foo(T z) __attribute__((weak)) : bar(2){}
390396
};
391397
};template <typename T, typename ...U>
392398
template <class V, int A>
393-
typename O1<T, U...>::template O2<V, A>::E O1<T, U...>::template O2<V, A>::I::foo(T, U..., V, E) { return E1; }
399+
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; }
394400
)cpp",
395401
""},
396402
// Destructors
@@ -399,6 +405,37 @@ typename O1<T, U...>::template O2<V, A>::E O1<T, U...>::template O2<V, A>::I::fo
399405
"class A { ~A(); };",
400406
"A::~A(){} ",
401407
},
408+
409+
// Member template
410+
{
411+
R"cpp(
412+
struct Foo {
413+
template <typename T, bool B = true>
414+
void ^bar() {}
415+
};)cpp",
416+
R"cpp(
417+
struct Foo {
418+
template <typename T, bool B = true>
419+
void bar() ;
420+
};template <typename T, bool B>
421+
inline void Foo::bar() {}
422+
)cpp",
423+
""},
424+
425+
// Class template with member template
426+
{
427+
R"cpp(
428+
template <typename T> struct Foo {
429+
template <typename U> void ^bar(const T& t, const U& u) {}
430+
};)cpp",
431+
R"cpp(
432+
template <typename T> struct Foo {
433+
template <typename U> void bar(const T& t, const U& u) ;
434+
};template <typename T>
435+
template <typename U>
436+
inline void Foo<T>::bar(const T& t, const U& u) {}
437+
)cpp",
438+
""},
402439
};
403440
for (const auto &Case : Cases) {
404441
SCOPED_TRACE(Case.Test);

0 commit comments

Comments
 (0)