Skip to content

[clangd] Let DefineOutline tweak handle member function templates #112345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 19, 2024
Merged

[clangd] Let DefineOutline tweak handle member function templates #112345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 48 additions & 19 deletions clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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;
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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()) {
Expand Down Expand Up @@ -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();

Expand Down
49 changes: 43 additions & 6 deletions clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down
Loading