Skip to content

Commit 5b729d0

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

File tree

2 files changed

+93
-22
lines changed

2 files changed

+93
-22
lines changed

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

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ 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());
@@ -130,23 +131,40 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
130131
return QualifiedFunc.takeError();
131132

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

164+
AddToTemplatePrefixIfApplicable(FD, true);
149165
auto Source = QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
166+
if (TargetFileIsHeader)
167+
Source.insert(0, "inline ");
150168
if (!TemplatePrefix.empty())
151169
Source.insert(0, TemplatePrefix);
152170
return Source;
@@ -202,7 +220,8 @@ deleteTokensWithKind(const syntax::TokenBuffer &TokBuf, tok::TokenKind Kind,
202220
llvm::Expected<std::string>
203221
getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
204222
const syntax::TokenBuffer &TokBuf,
205-
const HeuristicResolver *Resolver) {
223+
const HeuristicResolver *Resolver,
224+
bool targetFileIsHeader) {
206225
auto &AST = FD->getASTContext();
207226
auto &SM = AST.getSourceManager();
208227

@@ -337,7 +356,8 @@ getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
337356

338357
if (Errors)
339358
return std::move(Errors);
340-
return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);
359+
return getFunctionSourceAfterReplacements(FD, DeclarationCleanups,
360+
targetFileIsHeader);
341361
}
342362

343363
struct InsertionPoint {
@@ -419,15 +439,15 @@ class DefineOutline : public Tweak {
419439
Source->isOutOfLine())
420440
return false;
421441

422-
// Bail out if this is a function template or specialization, as their
442+
// Bail out if this is a function template specialization, as their
423443
// definitions need to be visible in all including translation units.
424-
if (Source->getDescribedFunctionTemplate())
425-
return false;
426444
if (Source->getTemplateSpecializationInfo())
427445
return false;
428446

429447
auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source);
430448
if (!MD) {
449+
if (Source->getDescribedFunctionTemplate())
450+
return false;
431451
// Can't outline free-standing functions in the same file.
432452
return !SameFile;
433453
}
@@ -443,13 +463,26 @@ class DefineOutline : public Tweak {
443463
SameFile = true;
444464

445465
// Bail out if the template parameter is unnamed.
466+
// FIXME: Is this really needed? It inhibits application on
467+
// e.g. std::enable_if.
446468
for (NamedDecl *P : *Params) {
447469
if (!P->getIdentifier())
448470
return false;
449471
}
450472
}
451473
}
452474

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

486519
auto FuncDef = getFunctionSourceCode(
487520
Source, InsertionPoint->EnclosingNamespace, Sel.AST->getTokens(),
488-
Sel.AST->getHeuristicResolver());
521+
Sel.AST->getHeuristicResolver(),
522+
SameFile && isHeaderFile(Sel.AST->tuPath(), Sel.AST->getLangOpts()));
489523
if (!FuncDef)
490524
return FuncDef.takeError();
491525

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