[PATCH] D26665: [clang-move] Support moving function.

[Eric Liu via cfe-commits]

ioeric added inline comments.


================
Comment at: clang-move/ClangMove.cpp:414
+  Optional<ast_matchers::internal::Matcher<NamedDecl>> InMovedSymbolNames;
   for (StringRef ClassName : Spec.Names) {
     llvm::StringRef GlobalClassName = ClassName.trim().ltrim(':');
----------------
s/ClassName/SymbolName/


================
Comment at: clang-move/ClangMove.cpp:417
     const auto HasName = hasName(("::" + GlobalClassName).str());
-    InMovedClassNames =
-        InMovedClassNames ? anyOf(*InMovedClassNames, HasName) : HasName;
+    InMovedSymbolNames =
+        InMovedSymbolNames ? anyOf(*InMovedSymbolNames, HasName) : HasName;
----------------
I guess `HasAnySymbolNames` would be better when used in the matchers below.


================
Comment at: test/clang-move/Inputs/function_test.h:1
+void f();
+
----------------
Maybe add a test case where both classes and functions exist.


================
Comment at: test/clang-move/move-function.cpp:9
+// CHECK-NEW-TEST-H-CASE1: #define {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-H-CASE1: inline int g() { return 0; }
+// CHECK-NEW-TEST-H-CASE1: #endif // {{.*}}NEW_FUNCTION_TEST_H
----------------
Shouldn't there be empty lines around this line?


================
Comment at: test/clang-move/move-function.cpp:18
+// CHECK-NEW-TEST-H-CASE2: #define {{.*}}NEW_FUNCTION_TEST_H
+// CHECK-NEW-TEST-H-CASE2: template <typename T> void h(T t) {}
+// CHECK-NEW-TEST-H-CASE2: template <> void h(int t) {}
----------------
Why does this become one line after being moved?

Shouldn't it be:
```
template<typename T>
void h(T t) {}
```


https://reviews.llvm.org/D26665