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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:395
 
     // Bail out in templated classes, as it is hard to spell the class name, i.e
     // if the template parameter is unnamed.
----------------
could you move this comment closer to the `isTempated` if statement below?


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:401
+
+      // The refactoring is meaningless for unnamed classes.
+      const auto *Parent = MD->getParent();
----------------
not just classes, but also for cases with unnamed namespaces.

could you change this to look like:
```
for(DeclContext *DC = MD->getParent(); DC; DC = DC->getParent()) {
  if (auto *ND = llvm::dyn_cast<NamedDecl>(DC)) {
     if(ND->getDeclName().isEmpty())
        return false;
  }
}
```


================
Comment at: clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp:97
+  // nesting.
+  EXPECT_UNAVAILABLE(R"cpp(
+    struct Foo {
----------------
can you also have a test inside an unnamed namespace?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143638