[PATCH] D147808: [clangd] Support defaulted destructors in Define Outline tweak

[Nathan James via Phabricator via cfe-commits]

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;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147808.511781.patch
Type: text/x-patch
Size: 2075 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230407/50725173/attachment.bin>