[clang-move] Better support enclosing class.

[hokein]

Summary

• When moving an outermost enclosing class, all its nested classes should also be moved together.
• Add a test for not moving nested class.

[ioeric]

clang-move/ClangMove.cpp

27 ↗

(On Diff #73928)

I'm not sure if we really need to limit this to the outer-most class. In the future, we might want to support moving nested class, so I guess this can simply be ofClass? Not sure if the name is right though.

343 ↗

(On Diff #73928)

What about static variable of the nested class?

[hokein]

support moving static member of nested class when moving its enclosing class.

[hokein]

clang-move/ClangMove.cpp

27 ↗

(On Diff #73928)

clang-move can't handle nested classes well enough currently, and supporting nested class requires more stuff there, like changing the qualifiers of the method definitions in the nested class (int A::B::f() should be int B::f() when moving nested class A::B). So we'd better to only allow outer-most class at least for now .

[ioeric]

lg

[hokein]

Rebase to master.

clang-tools-extra/trunk/clang-move/ClangMove.cpp

 #include "llvm/Support/Path.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace move {
 namespace {
 
+AST_MATCHER_P(Decl, hasOutermostEnclosingClass,
+              ast_matchers::internal::Matcher<Decl>, InnerMatcher) {
+  const auto* Context = Node.getDeclContext();
+  if (!Context) return false;
+  while (const auto *NextContext = Context->getParent()) {
+    if (isa<NamespaceDecl>(NextContext) ||
+        isa<TranslationUnitDecl>(NextContext))
+      break;
+    Context = NextContext;
+  }
+  return InnerMatcher.matches(*Decl::castFromDeclContext(Context), Finder,
+                              Builder);
+}
+
+AST_MATCHER_P(CXXMethodDecl, ofOutermostEnclosingClass,
+              ast_matchers::internal::Matcher<CXXRecordDecl>, InnerMatcher) {
+  const CXXRecordDecl *Parent = Node.getParent();
+  if (!Parent) return false;
+  while (const auto *NextParent =
+             dyn_cast<CXXRecordDecl>(Parent->getParent())) {
+    Parent = NextParent;
+  }
+
+  return InnerMatcher.matches(*Parent, Finder, Builder);
+}
+
 // Make the Path absolute using the CurrentDir if the Path is not an absolute
 // path. An empty Path will result in an empty string.
 std::string MakeAbsolutePath(StringRef CurrentDir, StringRef Path) {
   if (Path.empty())
     return "";
   llvm::SmallString<128> InitialDirectory(CurrentDir);
   llvm::SmallString<128> AbsolutePath(Path);
   if (std::error_code EC =
void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
   }
 
   auto InOldHeader = isExpansionInFile(
       MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader));
   auto InOldCC = isExpansionInFile(
       MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC));
   auto InOldFiles = anyOf(InOldHeader, InOldCC);
   auto InMovedClass =
-      hasDeclContext(cxxRecordDecl(*InMovedClassNames));
+      hasOutermostEnclosingClass(cxxRecordDecl(*InMovedClassNames));
 
   // Match moved class declarations.
   auto MovedClass = cxxRecordDecl(
       InOldFiles, *InMovedClassNames, isDefinition(),
       hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl())));
   Finder->addMatcher(MovedClass.bind("moved_class"), this);
 
   // Match moved class methods (static methods included) which are defined
   // outside moved class declaration.
-  Finder->addMatcher(cxxMethodDecl(InOldFiles,
-                                   ofClass(*InMovedClassNames),
-                                   isDefinition())
-                         .bind("class_method"),
-                     this);
+  Finder->addMatcher(
+      cxxMethodDecl(InOldFiles, ofOutermostEnclosingClass(*InMovedClassNames),
 
   // Match static member variable definition of the moved class.
   Finder->addMatcher(varDecl(InMovedClass, InOldCC, isDefinition())
                          .bind("class_static_var_decl"),
                      this);
 
   auto inAnonymousNamespace = hasParent(namespaceDecl(isAnonymous()));
   // Match functions/variables definitions which are defined in anonymous

clang-tools-extra/trunk/test/clang-move/Inputs/multiple_class_test.h

public:
   int f();
 };
 
 class Move4 {
 public:
   int f();
 };
 
+class EnclosingMove5 {
+public:
+  class Nested {
+    int f();
+    static int b;
+  };
+  static int a;
+};
+
 class NoMove {
 public:
   int f();
 };
 } // namespace c

clang-tools-extra/trunk/test/clang-move/Inputs/multiple_class_test.cpp

 int Move3::f() {
   return 0;
 }
 
 int Move4::f() {
   return 0;
 }
 
+int EnclosingMove5::a = 1;
+
+int EnclosingMove5::Nested::f() {
+  return 0;
+}
+
+int EnclosingMove5::Nested::b = 1;
+
 int NoMove::f() {
   return 0;
 }
 } // namespace c

clang-tools-extra/trunk/test/clang-move/move-multiple-classes.cpp

 // RUN: mkdir -p %T/move-multiple-classes
 // RUN: cp %S/Inputs/multiple_class_test*  %T/move-multiple-classes/
 // RUN: cd %T/move-multiple-classes
-// RUN: clang-move -names="a::Move1, b::Move2,c::Move3,c::Move4" -new_cc=%T/move-multiple-classes/new_multiple_class_test.cpp -new_header=%T/move-multiple-classes/new_multiple_class_test.h -old_cc=%T/move-multiple-classes/multiple_class_test.cpp -old_header=../move-multiple-classes/multiple_class_test.h %T/move-multiple-classes/multiple_class_test.cpp --
+// RUN: clang-move -names="c::EnclosingMove5::Nested" -new_cc=%T/move-multiple-classes/new_multiple_class_test.cpp -new_header=%T/move-multiple-classes/new_multiple_class_test.h -old_cc=%T/move-multiple-classes/multiple_class_test.cpp -old_header=../move-multiple-classes/multiple_class_test.h -dump_result %T/move-multiple-classes/multiple_class_test.cpp -- | FileCheck %s -check-prefix=CHECK-EMPTY
+// RUN: clang-move -names="a::Move1, b::Move2,c::Move3,c::Move4,c::EnclosingMove5" -new_cc=%T/move-multiple-classes/new_multiple_class_test.cpp -new_header=%T/move-multiple-classes/new_multiple_class_test.h -old_cc=%T/move-multiple-classes/multiple_class_test.cpp -old_header=../move-multiple-classes/multiple_class_test.h %T/move-multiple-classes/multiple_class_test.cpp --
 // RUN: FileCheck -input-file=%T/move-multiple-classes/new_multiple_class_test.cpp -check-prefix=CHECK-NEW-TEST-CPP %s
 // RUN: FileCheck -input-file=%T/move-multiple-classes/new_multiple_class_test.h -check-prefix=CHECK-NEW-TEST-H %s
 // RUN: FileCheck -input-file=%T/move-multiple-classes/multiple_class_test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
 // RUN: FileCheck -input-file=%T/move-multiple-classes/multiple_class_test.h -check-prefix=CHECK-OLD-TEST-H %s
 //
+// CHECK-EMPTY: [{{[[:space:]]*}}]
+//
 // CHECK-OLD-TEST-H: namespace c {
 // CHECK-OLD-TEST-H: class NoMove {
 // CHECK-OLD-TEST-H: public:
 // CHECK-OLD-TEST-H:   int f();
 // CHECK-OLD-TEST-H: };
 // CHECK-OLD-TEST-H: } // namespace c
 
 // CHECK-OLD-TEST-CPP: #include "{{.*}}multiple_class_test.h"

 // CHECK-NEW-TEST-H: class Move3 {
 // CHECK-NEW-TEST-H: public:
 // CHECK-NEW-TEST-H:   int f();
 // CHECK-NEW-TEST-H: };
 // CHECK-NEW-TEST-H: class Move4 {
 // CHECK-NEW-TEST-H: public:
 // CHECK-NEW-TEST-H:   int f();
 // CHECK-NEW-TEST-H: };
+// CHECK-NEW-TEST-H: class EnclosingMove5 {
+// CHECK-NEW-TEST-H: public:
+// CHECK-NEW-TEST-H:   class Nested {
+// CHECK-NEW-TEST-H:     int f();
+// CHECK-NEW-TEST-H:     static int b;
+// CHECK-NEW-TEST-H:   };
+// CHECK-NEW-TEST-H:   static int a;
+// CHECK-NEW-TEST-H: };
 // CHECK-NEW-TEST-H: } // namespace c
 
 // CHECK-NEW-TEST-CPP: #include "{{.*}}new_multiple_class_test.h"
 // CHECK-NEW-TEST-CPP: namespace a {
 // CHECK-NEW-TEST-CPP: int Move1::f() { return 0; }
 // CHECK-NEW-TEST-CPP: } // namespace a
 // CHECK-NEW-TEST-CPP: namespace b {
 // CHECK-NEW-TEST-CPP: int Move2::f() { return 0; }
 // CHECK-NEW-TEST-CPP: } // namespace b
 // CHECK-NEW-TEST-CPP: namespace c {
 // CHECK-NEW-TEST-CPP: int Move3::f() { return 0; }
 // CHECK-NEW-TEST-CPP: int Move4::f() { return 0; }
+// CHECK-NEW-TEST-CPP: int EnclosingMove5::a = 1;
+// CHECK-NEW-TEST-CPP: int EnclosingMove5::Nested::f() { return 0; }
+// CHECK-NEW-TEST-CPP: int EnclosingMove5::Nested::b = 1;
 // CHECK-NEW-TEST-CPP: } // namespace c