Skip to content

Commit 42ea89f

Browse files
ckandelerckandeler
committed
[clangd] Let DefineOutline tweak handle member function templates
1 parent 0a17bdf commit 42ea89f

File tree

2 files changed

+93
-24
lines changed

2 files changed

+93
-24
lines changed

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

Lines changed: 50 additions & 18 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());
@@ -130,23 +129,40 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
130129
return QualifiedFunc.takeError();
131130

132131
std::string TemplatePrefix;
132+
auto AddToTemplatePrefixIfApplicable = [&](const Decl *D, bool append) {
133+
const TemplateParameterList *Params = D->getDescribedTemplateParams();
134+
if (!Params)
135+
return;
136+
for (Decl *P : *Params) {
137+
if (auto *TTP = dyn_cast<TemplateTypeParmDecl>(P))
138+
TTP->removeDefaultArgument();
139+
else if (auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(P))
140+
NTTP->removeDefaultArgument();
141+
else if (auto *TTPD = dyn_cast<TemplateTemplateParmDecl>(P))
142+
TTPD->removeDefaultArgument();
143+
}
144+
std::string S;
145+
llvm::raw_string_ostream Stream(S);
146+
Params->print(Stream, FD->getASTContext());
147+
if (!S.empty())
148+
*S.rbegin() = '\n'; // Replace space with newline
149+
if (append)
150+
TemplatePrefix.append(S);
151+
else
152+
TemplatePrefix.insert(0, S);
153+
};
133154
if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(FD)) {
134155
for (const CXXRecordDecl *Parent = MD->getParent(); Parent;
135156
Parent =
136157
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-
}
158+
AddToTemplatePrefixIfApplicable(Parent, false);
146159
}
147160
}
148161

162+
AddToTemplatePrefixIfApplicable(FD, true);
149163
auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
164+
if (TargetFileIsHeader)
165+
Source.insert(0, "inline ");
150166
if (!TemplatePrefix.empty())
151167
Source.insert(0, TemplatePrefix);
152168
return Source;
@@ -202,7 +218,8 @@ deleteTokensWithKind(const syntax::TokenBuffer &TokBuf, tok::TokenKind Kind,
202218
llvm::Expected<std::string>
203219
getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
204220
const syntax::TokenBuffer &TokBuf,
205-
const HeuristicResolver *Resolver) {
221+
const HeuristicResolver *Resolver,
222+
bool targetFileIsHeader) {
206223
auto &AST = FD->getASTContext();
207224
auto &SM = AST.getSourceManager();
208225

@@ -337,7 +354,8 @@ getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
337354

338355
if (Errors)
339356
return std::move(Errors);
340-
return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);
357+
return getFunctionSourceAfterReplacements(FD, DeclarationCleanups,
358+
targetFileIsHeader);
341359
}
342360

343361
struct InsertionPoint {
@@ -419,15 +437,15 @@ class DefineOutline : public Tweak {
419437
Source->isOutOfLine())
420438
return false;
421439

422-
// Bail out if this is a function template or specialization, as their
440+
// Bail out if this is a function template specialization, as their
423441
// definitions need to be visible in all including translation units.
424-
if (Source->getDescribedFunctionTemplate())
425-
return false;
426442
if (Source->getTemplateSpecializationInfo())
427443
return false;
428444

429445
auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source);
430446
if (!MD) {
447+
if (Source->getDescribedFunctionTemplate())
448+
return false;
431449
// Can't outline free-standing functions in the same file.
432450
return !SameFile;
433451
}
@@ -443,13 +461,26 @@ class DefineOutline : public Tweak {
443461
SameFile = true;
444462

445463
// Bail out if the template parameter is unnamed.
464+
// FIXME: Is this really needed? It inhibits application on
465+
// e.g. std::enable_if.
446466
for (NamedDecl *P : *Params) {
447467
if (!P->getIdentifier())
448468
return false;
449469
}
450470
}
451471
}
452472

473+
// For function templates, the same limitations as for class templates
474+
// apply.
475+
if (const TemplateParameterList *Params =
476+
MD->getDescribedTemplateParams()) {
477+
for (NamedDecl *P : *Params) {
478+
if (!P->getIdentifier())
479+
return false;
480+
}
481+
SameFile = true;
482+
}
483+
453484
// The refactoring is meaningless for unnamed classes and namespaces,
454485
// unless we're outlining in the same file
455486
for (const DeclContext *DC = MD->getParent(); DC; DC = DC->getParent()) {
@@ -485,7 +516,8 @@ class DefineOutline : public Tweak {
485516

486517
auto FuncDef = getFunctionSourceCode(
487518
Source, InsertionPoint->EnclosingNamespace, Sel.AST->getTokens(),
488-
Sel.AST->getHeuristicResolver());
519+
Sel.AST->getHeuristicResolver(),
520+
SameFile && isHeaderFile(Sel.AST->tuPath(), Sel.AST->getLangOpts()));
489521
if (!FuncDef)
490522
return FuncDef.takeError();
491523

‎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)