[PATCH] D147802: [clangd] Handle destructors in DefineOutline tweak

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: kadircet, sammccall.
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.

Fix destructors being incorrectly defined in the DefineOutline tweak
Currently it doesn't prepend the class name to the destructor

  class A { ~A() {} };
  // Destructor definition after outline
  ~A() {}
  // After this fix
  A::~A() {}


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147802

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
@@ -319,6 +319,11 @@
             };)cpp",
           "  Foo::Foo(int) {}\n",
       },
+      { 
+        "class A { ~A^(){} };",
+        "class A { ~A(); };",
+        "A::~A(){} ",
+      },
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Test);
@@ -532,6 +537,18 @@
           // account. This can be spelled as b::foo instead.
           "using namespace a;void a::b::foo() {} ",
       },
+      { 
+        "namespace a { class A { ~A^(){} }; }",
+        "",
+        "namespace a { class A { ~A(); }; }",
+        "a::A::~A(){} ",
+      },
+      { 
+        "namespace a { class A { ~A^(){} }; }",
+        "namespace a{}",
+        "namespace a { class A { ~A(); }; }",
+        "namespace a{A::~A(){} }",
+      },
   };
   llvm::StringMap<std::string> EditedFiles;
   for (auto &Case : Cases) {
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
@@ -183,6 +183,15 @@
       },
       Resolver);
 
+  if (const auto *Destructor = llvm::dyn_cast<CXXDestructorDecl>(FD)) {
+    if (auto Err = DeclarationCleanups.add(tooling::Replacement(
+            SM, Destructor->getLocation(), 0,
+            getQualification(AST, *TargetContext,
+                             SM.getLocForStartOfFile(SM.getMainFileID()),
+                             Destructor))))
+      Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+  }
+
   // Get rid of default arguments, since they should not be specified in
   // out-of-line definition.
   for (const auto *PVD : FD->parameters()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147802.511755.patch
Type: text/x-patch
Size: 1999 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230407/0b8a72f8/attachment-0001.bin>