Skip to content

Commit 2eb9cc7

Browse files
committed
[clangd] Handle destructors in DefineOutline tweak
Fix destructors being incorrectly defined in the DefineOutline tweak Currently it doesn't prepend the class name to the destructor ```lang=c++ class A { ~A() {} }; // Destructor definition after outline ~A() {} // After this fix A::~A() {} ``` Reviewed By: kadircet Differential Revision: https://reviews.llvm.org/D147802
1 parent 88b8076 commit 2eb9cc7

File tree

2 files changed

+32
-0
lines changed

2 files changed

+32
-0
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,20 @@ getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace,
183183
},
184184
Resolver);
185185

186+
// findExplicitReferences doesn't provide references to
187+
// constructor/destructors, it only provides references to type names inside
188+
// them.
189+
// this works for constructors, but doesn't work for destructor as type name
190+
// doesn't cover leading `~`, so handle it specially.
191+
if (const auto *Destructor = llvm::dyn_cast<CXXDestructorDecl>(FD)) {
192+
if (auto Err = DeclarationCleanups.add(tooling::Replacement(
193+
SM, Destructor->getLocation(), 0,
194+
getQualification(AST, *TargetContext,
195+
SM.getLocForStartOfFile(SM.getMainFileID()),
196+
Destructor))))
197+
Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
198+
}
199+
186200
// Get rid of default arguments, since they should not be specified in
187201
// out-of-line definition.
188202
for (const auto *PVD : FD->parameters()) {

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,12 @@ TEST_F(DefineOutlineTest, ApplyTest) {
319319
};)cpp",
320320
" Foo::Foo(int) {}\n",
321321
},
322+
// Destrctors
323+
{
324+
"class A { ~A^(){} };",
325+
"class A { ~A(); };",
326+
"A::~A(){} ",
327+
},
322328
};
323329
for (const auto &Case : Cases) {
324330
SCOPED_TRACE(Case.Test);
@@ -532,6 +538,18 @@ TEST_F(DefineOutlineTest, QualifyFunctionName) {
532538
// account. This can be spelled as b::foo instead.
533539
"using namespace a;void a::b::foo() {} ",
534540
},
541+
{
542+
"namespace a { class A { ~A^(){} }; }",
543+
"",
544+
"namespace a { class A { ~A(); }; }",
545+
"a::A::~A(){} ",
546+
},
547+
{
548+
"namespace a { class A { ~A^(){} }; }",
549+
"namespace a{}",
550+
"namespace a { class A { ~A(); }; }",
551+
"namespace a{A::~A(){} }",
552+
},
535553
};
536554
llvm::StringMap<std::string> EditedFiles;
537555
for (auto &Case : Cases) {

0 commit comments

Comments
 (0)