[clang-move] Support moving function.

[ioeric]

clang-move/ClangMove.cpp

412–413

s/ClassName/SymbolName/

413–418

I guess HasAnySymbolNames would be better when used in the matchers below.

test/clang-move/Inputs/function_test.h

2

Maybe add a test case where both classes and functions exist.

test/clang-move/move-function.cpp

10

Shouldn't there be empty lines around this line?

19

Why does this become one line after being moved?

Shouldn't it be:

template<typename T>
void h(T t) {}

[hokein]

Add one more test and rename variables.

test/clang-move/move-function.cpp

10

There should be. FileCheck will skip empty lines and try to find next matching line here. Have added some \n checks.

19

formatAndApplyReplacements does this for us ,and this is actually LLVM code style...

[ioeric]

Lg with two nits.

clang-move/ClangMove.cpp

145

Maybe just inline this variable since this is only used once.

146

I'd assert(FD) instead.

[hokein]

Fix nits.

clang-move/ClangMove.cpp

if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc)))
                             FileEntry->getName(), FilenameRange, SM);
   }
 
 private:
   const SourceManager &SM;
   ClangMoveTool *const MoveTool;
 };
 
+class FunctionDeclarationMatch : public MatchFinder::MatchCallback {
+public:
+  explicit FunctionDeclarationMatch(ClangMoveTool *MoveTool)
+      : MoveTool(MoveTool) {}
+
+  void run(const MatchFinder::MatchResult &Result) override {
+    const auto *FD = Result.Nodes.getNodeAs<clang::FunctionDecl>("function");
+    assert(FD);
+    const clang::NamedDecl *D = FD;
+    if (const auto *FTD = FD->getDescribedFunctionTemplate())
+      D = FTD;
+    MoveTool->getMovedDecls().emplace_back(D,
+                                           &Result.Context->getSourceManager());
+    MoveTool->getUnremovedDeclsInOldHeader().erase(D);
+    MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back());
+  }
+
+private:
+  ClangMoveTool *MoveTool;
+};
+
 class ClassDeclarationMatch : public MatchFinder::MatchCallback {
 public:
   explicit ClassDeclarationMatch(ClangMoveTool *MoveTool)
       : MoveTool(MoveTool) {}
   void run(const MatchFinder::MatchResult &Result) override {
     clang::SourceManager* SM = &Result.Context->getSourceManager();
     if (const auto *CMD =
             Result.Nodes.getNodeAs<clang::CXXMethodDecl>("class_method"))
ClangMoveTool::ClangMoveTool(
     llvm::StringRef OriginalRunningDirectory, llvm::StringRef FallbackStyle)
     : Spec(MoveSpec), FileToReplacements(FileToReplacements),
       OriginalRunningDirectory(OriginalRunningDirectory),
       FallbackStyle(FallbackStyle) {
   if (!Spec.NewHeader.empty())
     CCIncludes.push_back("#include \"" + Spec.NewHeader + "\"\n");
 }
 
 void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  Optional<ast_matchers::internal::Matcher<NamedDecl>> InMovedClassNames;
-  for (StringRef ClassName : Spec.Names) {
-    llvm::StringRef GlobalClassName = ClassName.trim().ltrim(':');
-    const auto HasName = hasName(("::" + GlobalClassName).str());
-    InMovedClassNames =
-        InMovedClassNames ? anyOf(*InMovedClassNames, HasName) : HasName;
+  Optional<ast_matchers::internal::Matcher<NamedDecl>> HasAnySymbolNames;
+  for (StringRef SymbolName: Spec.Names) {
+    llvm::StringRef GlobalSymbolName = SymbolName.trim().ltrim(':');
+    const auto HasName = hasName(("::" + GlobalSymbolName).str());
+    HasAnySymbolNames =
+        HasAnySymbolNames ? anyOf(*HasAnySymbolNames, HasName) : HasName;
   }
-  if (!InMovedClassNames) {
-    llvm::errs() << "No classes being moved.\n";
+  if (!HasAnySymbolNames) {
+    llvm::errs() << "No symbols being moved.\n";
     return;
   }
 
   auto InOldHeader = isExpansionInFile(makeAbsolutePath(Spec.OldHeader));
   auto InOldCC = isExpansionInFile(makeAbsolutePath(Spec.OldCC));
   auto InOldFiles = anyOf(InOldHeader, InOldCC);
   auto InMovedClass =
-      hasOutermostEnclosingClass(cxxRecordDecl(*InMovedClassNames));
+      hasOutermostEnclosingClass(cxxRecordDecl(*HasAnySymbolNames));
 
   auto ForwardDecls =
       cxxRecordDecl(unless(anyOf(isImplicit(), isDefinition())));
 
   //============================================================================
   // Matchers for old header
   //============================================================================
   // Match all top-level named declarations (e.g. function, variable, enum) in
void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
   //============================================================================
   // Matchers for old files, including old.h/old.cc
   //============================================================================
   // Create a MatchCallback for class declarations.
   MatchCallbacks.push_back(llvm::make_unique<ClassDeclarationMatch>(this));
   // Match moved class declarations.
   auto MovedClass =
       cxxRecordDecl(
-          InOldFiles, *InMovedClassNames, isDefinition(),
+          InOldFiles, *HasAnySymbolNames, isDefinition(),
           hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl())))
           .bind("moved_class");
   Finder->addMatcher(MovedClass, MatchCallbacks.back().get());
   // Match moved class methods (static methods included) which are defined
   // outside moved class declaration.
   Finder->addMatcher(
-      cxxMethodDecl(InOldFiles, ofOutermostEnclosingClass(*InMovedClassNames),
+      cxxMethodDecl(InOldFiles, ofOutermostEnclosingClass(*HasAnySymbolNames),
                     isDefinition())
           .bind("class_method"),
       MatchCallbacks.back().get());
   // Match static member variable definition of the moved class.
   Finder->addMatcher(
       varDecl(InMovedClass, InOldFiles, isDefinition(), isStaticDataMember())
           .bind("class_static_var_decl"),
       MatchCallbacks.back().get());
 
+  MatchCallbacks.push_back(llvm::make_unique<FunctionDeclarationMatch>(this));
+  Finder->addMatcher(functionDecl(InOldFiles, *HasAnySymbolNames,
+                                  anyOf(hasDeclContext(namespaceDecl()),
+                                        hasDeclContext(translationUnitDecl())))
+                         .bind("function"),
+                     MatchCallbacks.back().get());
 }
 
 void ClangMoveTool::run(const ast_matchers::MatchFinder::MatchResult &Result) {
   if (const auto *D =
           Result.Nodes.getNodeAs<clang::NamedDecl>("decls_in_header")) {
     UnremovedDeclsInOldHeader.insert(D);
   } else if (const auto *FWD =
                  Result.Nodes.getNodeAs<clang::CXXRecordDecl>("fwd_decl")) {

test/clang-move/Inputs/function_test.h

+void f();
+
+inline int g() { return 0; }
+
+template<typename T>
+void h(T t) {}
+
+template<>
+void h(int t) {}
+
+class A {
+ public:
+  void f();
+};

test/clang-move/Inputs/function_test.cpp

+#include "function_test.h"
+
+void f() {}
+
+void A::f() {}

test/clang-move/move-function.cpp

+// RUN: mkdir -p %T/move-function
+// RUN: cp %S/Inputs/function_test*  %T/move-function
+// RUN: cd %T/move-function
+// RUN: clang-move -names="g" -new_header=%T/move-function/new_function_test.h -old_header=../move-function/function_test.h %T/move-function/function_test.cpp --
+// RUN: FileCheck -input-file=%T/move-function/new_function_test.h -check-prefix=CHECK-NEW-TEST-H-CASE1 %s
+//
+// CHECK-NEW-TEST-H-CASE1: #ifndef {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-H-CASE1: #define {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-H-CASE1: {{[[:space:]]+}}
+// CHECK-NEW-TEST-H-CASE1: inline int g() { return 0; }
+// CHECK-NEW-TEST-H-CASE1: {{[[:space:]]+}}
+// CHECK-NEW-TEST-H-CASE1: #endif // {{.*}}NEW_FUNCTION_TEST_H
+//
+// RUN: cp %S/Inputs/function_test*  %T/move-function
+// RUN: clang-move -names="h" -new_header=%T/move-function/new_function_test.h -old_header=../move-function/function_test.h %T/move-function/function_test.cpp --
+// RUN: FileCheck -input-file=%T/move-function/new_function_test.h -check-prefix=CHECK-NEW-TEST-H-CASE2 %s
+//
+// CHECK-NEW-TEST-H-CASE2: #ifndef {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-H-CASE2: #define {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-H-CASE2: {{[[:space:]]+}}
+// CHECK-NEW-TEST-H-CASE2: template <typename T> void h(T t) {}
+// CHECK-NEW-TEST-H-CASE2: {{[[:space:]]+}}
+// CHECK-NEW-TEST-H-CASE2: template <> void h(int t) {}
+// CHECK-NEW-TEST-H-CASE2: {{[[:space:]]+}}
+// CHECK-NEW-TEST-H-CASE2: #endif // {{.*}}NEW_FUNCTION_TEST_H
+//
+// RUN: cp %S/Inputs/function_test*  %T/move-function
+// RUN: clang-move -names="f" -new_header=%T/move-function/new_function_test.h -new_cc=%T/move-function/new_function_test.cpp -old_header=../move-function/function_test.h -old_cc=../move-function/function_test.cpp %T/move-function/function_test.cpp --
+// RUN: FileCheck -input-file=%T/move-function/new_function_test.h -check-prefix=CHECK-NEW-TEST-H-CASE3 %s
+// RUN: FileCheck -input-file=%T/move-function/new_function_test.cpp -check-prefix=CHECK-NEW-TEST-CPP-CASE3 %s
+//
+// CHECK-NEW-TEST-H-CASE3: #ifndef {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-H-CASE3: #define {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-H-CASE3: {{[[:space:]]+}}
+// CHECK-NEW-TEST-H-CASE3: void f();
+// CHECK-NEW-TEST-H-CASE3: {{[[:space:]]+}}
+// CHECK-NEW-TEST-H-CASE3: #endif // {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-CPP-CASE3: #include "{{.*}}new_function_test.h"
+// CHECK-NEW-TEST-CPP-CASE3: {{[[:space:]]+}}
+// CHECK-NEW-TEST-CPP-CASE3: void f() {}
+//
+// RUN: cp %S/Inputs/function_test*  %T/move-function
+// RUN: clang-move -names="A::f" -new_header=%T/move-function/new_function_test.h -new_cc=%T/move-function/new_function_test.cpp -old_header=../move-function/function_test.h -old_cc=../move-function/function_test.cpp %T/move-function/function_test.cpp -dump_result -- | FileCheck %s -check-prefix=CHECK-EMPTY
+//
+// CHECK-EMPTY: [{{[[:space:]]*}}]
+//
+// RUN: cp %S/Inputs/function_test*  %T/move-function
+// RUN: clang-move -names="f,A" -new_header=%T/move-function/new_function_test.h -new_cc=%T/move-function/new_function_test.cpp -old_header=../move-function/function_test.h -old_cc=../move-function/function_test.cpp %T/move-function/function_test.cpp --
+// RUN: FileCheck -input-file=%T/move-function/new_function_test.h -check-prefix=CHECK-NEW-TEST-H-CASE4 %s
+// RUN: FileCheck -input-file=%T/move-function/new_function_test.cpp -check-prefix=CHECK-NEW-TEST-CPP-CASE4 %s
+
+// CHECK-NEW-TEST-H-CASE4: #ifndef {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-H-CASE4: #define {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-H-CASE4: {{[[:space:]]+}}
+// CHECK-NEW-TEST-H-CASE4: void f();
+// CHECK-NEW-TEST-H-CASE4: {{[[:space:]]+}}
+// CHECK-NEW-TEST-H-CASE4: class A {
+// CHECK-NEW-TEST-H-CASE4: public:
+// CHECK-NEW-TEST-H-CASE4:   void f();
+// CHECK-NEW-TEST-H-CASE4: };
+// CHECK-NEW-TEST-H-CASE4: {{[[:space:]]+}}
+// CHECK-NEW-TEST-H-CASE4: #endif // {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-CPP-CASE4: #include "{{.*}}new_function_test.h"
+// CHECK-NEW-TEST-CPP-CASE4: {{[[:space:]]+}}
+// CHECK-NEW-TEST-CPP-CASE4: void f() {}
+// CHECK-NEW-TEST-CPP-CASE4: {{[[:space:]]+}}
+// CHECK-NEW-TEST-CPP-CASE4: void A::f() {}