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

[Denis Fatkulin via Phabricator via cfe-commits]

denis-fatkulin marked 3 inline comments as done.
denis-fatkulin 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.
----------------
kadircet wrote:
> could you move this comment closer to the `isTempated` if statement below?
Fixed


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:401
+
+      // The refactoring is meaningless for unnamed classes.
+      const auto *Parent = MD->getParent();
----------------
kadircet wrote:
> 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;
>   }
> }
> ```
It's a useful remark. Tanks! 


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143638