njames93 created this revision.
njames93 added reviewers: sammccall, kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Sometimes classes can't have a defaulted or empty destructor declared inside
the class if any of the members have forward declared types as template
parameters.
The Pimpl idiom is a classic example of this
In these situations it makes sense to have a defaulted destructor moved to the
source file. Currently the define outline tweak won't will reject these
defaulted destructors.
Depends on D147802 <https://reviews.llvm.org/D147802>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147808
Files:
clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
Index: clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -325,6 +325,11 @@
"class A { ~A(); };",
"A::~A(){} ",
},
+ {
+ "class A { ~A^() = default; };",
+ "class A { ~A() ; };",
+ "A::~A() = default;",
+ },
};
for (const auto &Case : Cases) {
SCOPED_TRACE(Case.Test);
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -320,6 +320,14 @@
// initializers.
SourceRange getDeletionRange(const FunctionDecl *FD,
const syntax::TokenBuffer &TokBuf) {
+ if (FD->isExplicitlyDefaulted()) {
+ auto Toks =
+ TokBuf.expandedTokens({FD->getTypeSpecEndLoc(), FD->getDefaultLoc()});
+ Toks = Toks.drop_until(
+ [](const syntax::Token &Tok) { return Tok.kind() == tok::equal; });
+ assert(Toks.size() == 2);
+ return {Toks.front().location(), Toks.back().endLocation()};
+ }
auto DeletionRange = FD->getBody()->getSourceRange();
if (auto *CD = llvm::dyn_cast<CXXConstructorDecl>(FD)) {
// AST doesn't contain the location for ":" in ctor initializers. Therefore
@@ -390,7 +398,12 @@
Source = getSelectedFunction(Sel.ASTSelection.commonAncestor());
// Bail out if the selection is not a in-line function definition.
- if (!Source || !Source->doesThisDeclarationHaveABody() ||
+ // Defaulted destrucors are OK as they sometimes need to be defined out of
+ // line.
+ if (!Source ||
+ (!Source->doesThisDeclarationHaveABody() &&
+ (!isa<CXXDestructorDecl>(Source) ||
+ !Source->isExplicitlyDefaulted())) ||
Source->isOutOfLine())
return false;
Index: clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -325,6 +325,11 @@
"class A { ~A(); };",
"A::~A(){} ",
},
+ {
+ "class A { ~A^() = default; };",
+ "class A { ~A() ; };",
+ "A::~A() = default;",
+ },
};
for (const auto &Case : Cases) {
SCOPED_TRACE(Case.Test);
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -320,6 +320,14 @@
// initializers.
SourceRange getDeletionRange(const FunctionDecl *FD,
const syntax::TokenBuffer &TokBuf) {
+ if (FD->isExplicitlyDefaulted()) {
+ auto Toks =
+ TokBuf.expandedTokens({FD->getTypeSpecEndLoc(), FD->getDefaultLoc()});
+ Toks = Toks.drop_until(
+ [](const syntax::Token &Tok) { return Tok.kind() == tok::equal; });
+ assert(Toks.size() == 2);
+ return {Toks.front().location(), Toks.back().endLocation()};
+ }
auto DeletionRange = FD->getBody()->getSourceRange();
if (auto *CD = llvm::dyn_cast<CXXConstructorDecl>(FD)) {
// AST doesn't contain the location for ":" in ctor initializers. Therefore
@@ -390,7 +398,12 @@
Source = getSelectedFunction(Sel.ASTSelection.commonAncestor());
// Bail out if the selection is not a in-line function definition.
- if (!Source || !Source->doesThisDeclarationHaveABody() ||
+ // Defaulted destrucors are OK as they sometimes need to be defined out of
+ // line.
+ if (!Source ||
+ (!Source->doesThisDeclarationHaveABody() &&
+ (!isa<CXXDestructorDecl>(Source) ||
+ !Source->isExplicitlyDefaulted())) ||
Source->isOutOfLine())
return false;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
