hokein updated this revision to Diff 73819.
hokein marked 7 inline comments as done.
hokein added a comment.
Address review comments.
https://reviews.llvm.org/D25309
Files:
clang-move/ClangMove.cpp
clang-move/ClangMove.h
clang-move/tool/ClangMoveMain.cpp
test/clang-move/Inputs/database_template.json
test/clang-move/Inputs/multiple_class_test.cpp
test/clang-move/Inputs/multiple_class_test.h
test/clang-move/move-class.cpp
test/clang-move/move-multiple-classes.cpp
unittests/clang-move/ClangMoveTests.cpp
Index: unittests/clang-move/ClangMoveTests.cpp
===================================================================
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -210,7 +210,7 @@
TEST(ClangMove, MoveHeaderAndCC) {
move::ClangMoveTool::MoveDefinitionSpec Spec;
- Spec.Name = "a::b::Foo";
+ Spec.Names = "a::b::Foo";
Spec.OldHeader = "foo.h";
Spec.OldCC = "foo.cc";
Spec.NewHeader = "new_foo.h";
@@ -225,7 +225,7 @@
TEST(ClangMove, MoveHeaderOnly) {
move::ClangMoveTool::MoveDefinitionSpec Spec;
- Spec.Name = "a::b::Foo";
+ Spec.Names = "a::b::Foo";
Spec.OldHeader = "foo.h";
Spec.NewHeader = "new_foo.h";
auto Results = runClangMoveOnCode(Spec);
@@ -236,7 +236,7 @@
TEST(ClangMove, MoveCCOnly) {
move::ClangMoveTool::MoveDefinitionSpec Spec;
- Spec.Name = "a::b::Foo";
+ Spec.Names = "a::b::Foo";
Spec.OldCC = "foo.cc";
Spec.NewCC = "new_foo.cc";
std::string ExpectedHeader = "#include \"foo.h\"\n\n";
@@ -248,7 +248,7 @@
TEST(ClangMove, MoveNonExistClass) {
move::ClangMoveTool::MoveDefinitionSpec Spec;
- Spec.Name = "NonExistFoo";
+ Spec.Names = "NonExistFoo";
Spec.OldHeader = "foo.h";
Spec.OldCC = "foo.cc";
Spec.NewHeader = "new_foo.h";
Index: test/clang-move/move-multiple-classes.cpp
===================================================================
--- /dev/null
+++ test/clang-move/move-multiple-classes.cpp
@@ -0,0 +1,53 @@
+// RUN: mkdir -p %T/clang-move/build
+// RUN: sed 's|$test_dir|%/T/clang-move|g' %S/Inputs/database_template.json > %T/clang-move/compile_commands.json
+// RUN: cp %S/Inputs/multiple_class_test* %T/clang-move/
+// RUN: cd %T/clang-move
+// RUN: clang-move -names="a::Move1, b::Move2,c::Move3" -new_cc=%T/clang-move/new_multiple_class_test.cpp -new_header=%T/clang-move/new_multiple_class_test.h -old_cc=%T/clang-move/multiple_class_test.cpp -old_header=../clang-move/multiple_class_test.h %T/clang-move/multiple_class_test.cpp
+// RUN: FileCheck -input-file=%T/clang-move/new_multiple_class_test.cpp -check-prefix=CHECK-NEW-TEST-CPP %s
+// RUN: FileCheck -input-file=%T/clang-move/new_multiple_class_test.h -check-prefix=CHECK-NEW-TEST-H %s
+// RUN: FileCheck -input-file=%T/clang-move/multiple_class_test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
+// RUN: FileCheck -input-file=%T/clang-move/multiple_class_test.h -check-prefix=CHECK-OLD-TEST-H %s
+//
+// 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-OLD-TEST-CPP: namespace c {
+// CHECK-OLD-TEST-CPP: int NoMove::f() {
+// CHECK-OLD-TEST-CPP: return 0;
+// CHECK-OLD-TEST-CPP: }
+// CHECK-OLD-TEST-CPP: } // namespace c
+
+// CHECK-NEW-TEST-H: namespace a {
+// CHECK-NEW-TEST-H: class Move1 {
+// CHECK-NEW-TEST-H: public:
+// CHECK-NEW-TEST-H: int f();
+// CHECK-NEW-TEST-H: };
+// CHECK-NEW-TEST-H: } // namespace a
+// CHECK-NEW-TEST-H: namespace b {
+// CHECK-NEW-TEST-H: class Move2 {
+// CHECK-NEW-TEST-H: public:
+// CHECK-NEW-TEST-H: int f();
+// CHECK-NEW-TEST-H: };
+// CHECK-NEW-TEST-H: } // namespace b
+// CHECK-NEW-TEST-H: namespace c {
+// 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: } // 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: } // namespace c
Index: test/clang-move/move-class.cpp
===================================================================
--- test/clang-move/move-class.cpp
+++ test/clang-move/move-class.cpp
@@ -3,15 +3,15 @@
// RUN: cp %S/Inputs/test* %T/clang-move/
// RUN: touch %T/clang-move/test2.h
// RUN: cd %T/clang-move
-// RUN: clang-move -name="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=../clang-move/test.cpp -old_header=../clang-move/test.h %T/clang-move/test.cpp
+// RUN: clang-move -names="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=../clang-move/test.cpp -old_header=../clang-move/test.h %T/clang-move/test.cpp
// RUN: FileCheck -input-file=%T/clang-move/new_test.cpp -check-prefix=CHECK-NEW-TEST-CPP %s
// RUN: FileCheck -input-file=%T/clang-move/new_test.h -check-prefix=CHECK-NEW-TEST-H %s
// RUN: FileCheck -input-file=%T/clang-move/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
// RUN: FileCheck -input-file=%T/clang-move/test.h %s -implicit-check-not='{{namespace.*}}'
//
// RUN: cp %S/Inputs/test* %T/clang-move/
// RUN: cd %T/clang-move
-// RUN: clang-move -name="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=%T/clang-move/test.cpp -old_header=%T/clang-move/test.h %T/clang-move/test.cpp
+// RUN: clang-move -names="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=%T/clang-move/test.cpp -old_header=%T/clang-move/test.h %T/clang-move/test.cpp
// RUN: FileCheck -input-file=%T/clang-move/new_test.cpp -check-prefix=CHECK-NEW-TEST-CPP %s
// RUN: FileCheck -input-file=%T/clang-move/new_test.h -check-prefix=CHECK-NEW-TEST-H %s
// RUN: FileCheck -input-file=%T/clang-move/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
Index: test/clang-move/Inputs/multiple_class_test.h
===================================================================
--- /dev/null
+++ test/clang-move/Inputs/multiple_class_test.h
@@ -0,0 +1,25 @@
+namespace a {
+class Move1 {
+public:
+ int f();
+};
+} // namespace a
+
+namespace b {
+class Move2 {
+public:
+ int f();
+};
+} // namespace b
+
+namespace c {
+class Move3 {
+public:
+ int f();
+};
+
+class NoMove {
+public:
+ int f();
+};
+} // namespace c
Index: test/clang-move/Inputs/multiple_class_test.cpp
===================================================================
--- /dev/null
+++ test/clang-move/Inputs/multiple_class_test.cpp
@@ -0,0 +1,22 @@
+#include "multiple_class_test.h"
+
+namespace a {
+int Move1::f() {
+ return 0;
+}
+} // namespace a
+
+namespace b {
+int Move2::f() {
+ return 0;
+}
+} // namespace b
+
+namespace c {
+int Move3::f() {
+ return 0;
+}
+int NoMove::f() {
+ return 0;
+}
+} // namespace c
Index: test/clang-move/Inputs/database_template.json
===================================================================
--- test/clang-move/Inputs/database_template.json
+++ test/clang-move/Inputs/database_template.json
@@ -3,5 +3,10 @@
"directory": "$test_dir/build",
"command": "clang++ -o test.o $test_dir/test.cpp",
"file": "$test_dir/test.cpp"
+},
+{
+ "directory": "$test_dir/build",
+ "command": "clang++ -o test.o $test_dir/multiple_class_test.cpp",
+ "file": "$test_dir/multiple_class_test.cpp"
}
]
Index: clang-move/tool/ClangMoveMain.cpp
===================================================================
--- clang-move/tool/ClangMoveMain.cpp
+++ clang-move/tool/ClangMoveMain.cpp
@@ -37,8 +37,10 @@
cl::OptionCategory ClangMoveCategory("clang-move options");
-cl::opt<std::string> Name("name", cl::desc("The name of class being moved."),
- cl::cat(ClangMoveCategory));
+cl::opt<std::string>
+ Names("names", cl::desc("A comma-separated list of the names of classes "
+ "being moved, e.g. \"Foo\", \"a::Foo, b::Foo\"."),
+ cl::cat(ClangMoveCategory));
cl::opt<std::string>
OldHeader("old_header",
@@ -86,7 +88,7 @@
tooling::RefactoringTool Tool(OptionsParser.getCompilations(),
OptionsParser.getSourcePathList());
move::ClangMoveTool::MoveDefinitionSpec Spec;
- Spec.Name = Name;
+ Spec.Names = Names;
Spec.OldHeader = OldHeader;
Spec.NewHeader = NewHeader;
Spec.OldCC = OldCC;
Index: clang-move/ClangMove.h
===================================================================
--- clang-move/ClangMove.h
+++ clang-move/ClangMove.h
@@ -37,8 +37,9 @@
};
struct MoveDefinitionSpec {
- // A fully qualified name, e.g. "X", "a::X".
- std::string Name;
+ // A comma-separated list of fully qualified names, e.g. "Foo",
+ // "a::Foo, b::Foo".
+ std::string Names;
// The file path of old header, can be relative path and absolute path.
std::string OldHeader;
// The file path of old cc, can be relative path and absolute path.
Index: clang-move/ClangMove.cpp
===================================================================
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -287,31 +287,43 @@
: Spec(MoveSpec), FileToReplacements(FileToReplacements),
OriginalRunningDirectory(OriginalRunningDirectory),
FallbackStyle(FallbackStyle) {
- Spec.Name = llvm::StringRef(Spec.Name).ltrim(':');
if (!Spec.NewHeader.empty())
CCIncludes.push_back("#include \"" + Spec.NewHeader + "\"\n");
}
void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
- std::string FullyQualifiedName = "::" + Spec.Name;
+ SmallVector<StringRef, 4> ClassNames;
+ llvm::StringRef(Spec.Names).split(ClassNames, ',');
+ Optional<ast_matchers::internal::Matcher<NamedDecl>> InMovedClassNames;
+ for (StringRef ClassName : ClassNames) {
+ // Removed the preceding and trailing space of ClassName.
+ // And removed the global scope operator "::" since we always regard the
+ // ClassName as a global name.
+ llvm::StringRef GlobalClassName = ClassName.trim(' ').ltrim(':');
+ const auto HasName = hasName(("::" + GlobalClassName).str());
+ InMovedClassNames =
+ InMovedClassNames ? anyOf(*InMovedClassNames, HasName) : HasName;
+ }
+ assert(InMovedClassNames);
+
auto InOldHeader = isExpansionInFile(
MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader));
auto InOldCC = isExpansionInFile(
MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC));
auto InOldFiles = anyOf(InOldHeader, InOldCC);
auto InMovedClass =
- hasDeclContext(cxxRecordDecl(hasName(FullyQualifiedName)));
+ hasDeclContext(cxxRecordDecl(*InMovedClassNames));
// Match moved class declarations.
auto MovedClass = cxxRecordDecl(
- InOldFiles, hasName(FullyQualifiedName), isDefinition(),
+ 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(hasName(FullyQualifiedName)),
+ ofClass(*InMovedClassNames),
isDefinition())
.bind("class_method"),
this);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
