[PATCH] D143638: [clangd] Move function body to out-of-line: unnamed class method incorrect moving

[Denis Fatkulin via Phabricator via cfe-commits]

denis-fatkulin updated this revision to Diff 496375.
denis-fatkulin added a comment.

Review fixes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143638/new/

https://reviews.llvm.org/D143638

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
@@ -84,6 +84,32 @@
     template <typename> void fo^o() {};
     template <> void fo^o<int>() {};
   )cpp");
+
+  // Not available on methods of unnamed classes.
+  EXPECT_UNAVAILABLE(R"cpp(
+    struct Foo {
+      struct { void b^ar() {} } Bar;
+    };
+  )cpp");
+
+  // Not available on methods of named classes with unnamed parent in parents
+  // nesting.
+  EXPECT_UNAVAILABLE(R"cpp(
+    struct Foo {
+      struct {
+        struct Bar { void b^ar() {} };
+      } Baz;
+    };
+  )cpp");
+
+  // Not available on definitions within unnamed namespaces
+  EXPECT_UNAVAILABLE(R"cpp(
+    namespace {
+      struct Foo {
+        void f^oo() {}
+      };
+    } // namespace
+  )cpp");
 }
 
 TEST_F(DefineOutlineTest, FailsWithoutSource) {
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
@@ -392,11 +392,20 @@
     if (Source->getTemplateSpecializationInfo())
       return false;
 
-    // Bail out in templated classes, as it is hard to spell the class name, 
i.e
-    // if the template parameter is unnamed.
     if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source)) {
+      // Bail out in templated classes, as it is hard to spell the class name,
+      // i.e if the template parameter is unnamed.
       if (MD->getParent()->isTemplated())
         return false;
+
+      // The refactoring is meaningless for unnamed classes and definitions
+      // within unnamed namespaces.
+      for (const DeclContext *DC = MD->getParent(); DC; DC = DC->getParent()) {
+        if (auto *ND = llvm::dyn_cast<NamedDecl>(DC)) {
+          if (ND->getDeclName().isEmpty())
+            return false;
+        }
+      }
     }
 
     // Note that we don't check whether an implementation file exists or not in



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
@@ -84,6 +84,32 @@
     template <typename> void fo^o() {};
     template <> void fo^o<int>() {};
   )cpp");
+
+  // Not available on methods of unnamed classes.
+  EXPECT_UNAVAILABLE(R"cpp(
+    struct Foo {
+      struct { void b^ar() {} } Bar;
+    };
+  )cpp");
+
+  // Not available on methods of named classes with unnamed parent in parents
+  // nesting.
+  EXPECT_UNAVAILABLE(R"cpp(
+    struct Foo {
+      struct {
+        struct Bar { void b^ar() {} };
+      } Baz;
+    };
+  )cpp");
+
+  // Not available on definitions within unnamed namespaces
+  EXPECT_UNAVAILABLE(R"cpp(
+    namespace {
+      struct Foo {
+        void f^oo() {}
+      };
+    } // namespace
+  )cpp");
 }
 
 TEST_F(DefineOutlineTest, FailsWithoutSource) {
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
@@ -392,11 +392,20 @@
     if (Source->getTemplateSpecializationInfo())
       return false;
 
-    // Bail out in templated classes, as it is hard to spell the class name, i.e
-    // if the template parameter is unnamed.
     if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source)) {
+      // Bail out in templated classes, as it is hard to spell the class name,
+      // i.e if the template parameter is unnamed.
       if (MD->getParent()->isTemplated())
         return false;
+
+      // The refactoring is meaningless for unnamed classes and definitions
+      // within unnamed namespaces.
+      for (const DeclContext *DC = MD->getParent(); DC; DC = DC->getParent()) {
+        if (auto *ND = llvm::dyn_cast<NamedDecl>(DC)) {
+          if (ND->getDeclName().isEmpty())
+            return false;
+        }
+      }
     }
 
     // Note that we don't check whether an implementation file exists or not in

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits