[PATCH] D25586: [clang-move] Use cl::list and cl::CommaSeparated for the list of names.

[Alexander Shaposhnikov via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL284291: [clang-move] Use cl::list for the list of names 
(authored by alexshap).

Changed prior to commit:
  https://reviews.llvm.org/D25586?vs=74599=74749#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25586

Files:
  clang-tools-extra/trunk/clang-move/ClangMove.cpp
  clang-tools-extra/trunk/clang-move/ClangMove.h
  clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp
  clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp


Index: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
===
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
@@ -213,7 +213,7 @@
 
 TEST(ClangMove, MoveHeaderAndCC) {
   move::ClangMoveTool::MoveDefinitionSpec Spec;
-  Spec.Names = "a::b::Foo";
+  Spec.Names = { "a::b::Foo" };
   Spec.OldHeader = "foo.h";
   Spec.OldCC = "foo.cc";
   Spec.NewHeader = "new_foo.h";
@@ -228,7 +228,7 @@
 
 TEST(ClangMove, MoveHeaderOnly) {
   move::ClangMoveTool::MoveDefinitionSpec Spec;
-  Spec.Names = "a::b::Foo";
+  Spec.Names = { "a::b::Foo" };
   Spec.OldHeader = "foo.h";
   Spec.NewHeader = "new_foo.h";
   auto Results = runClangMoveOnCode(Spec);
@@ -239,7 +239,7 @@
 
 TEST(ClangMove, MoveCCOnly) {
   move::ClangMoveTool::MoveDefinitionSpec Spec;
-  Spec.Names = "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";
@@ -251,7 +251,7 @@
 
 TEST(ClangMove, MoveNonExistClass) {
   move::ClangMoveTool::MoveDefinitionSpec Spec;
-  Spec.Names = "NonExistFoo";
+  Spec.Names = { "NonExistFoo" };
   Spec.OldHeader = "foo.h";
   Spec.OldCC = "foo.cc";
   Spec.NewHeader = "new_foo.h";
Index: clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp
===
--- clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp
+++ clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp
@@ -38,10 +38,10 @@
 
 cl::OptionCategory ClangMoveCategory("clang-move options");
 
-cl::opt
-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::list Names("names", cl::CommaSeparated, cl::OneOrMore,
+cl::desc("The list of the names of classes being "
+ "moved, e.g. \"Foo,a::Foo,b::Foo\"."),
+cl::cat(ClangMoveCategory));
 
 cl::opt
 OldHeader("old_header",
@@ -90,7 +90,7 @@
   tooling::RefactoringTool Tool(OptionsParser.getCompilations(),
 OptionsParser.getSourcePathList());
   move::ClangMoveTool::MoveDefinitionSpec Spec;
-  Spec.Names = Names;
+  Spec.Names = { Names.begin(), Names.end() };
   Spec.OldHeader = OldHeader;
   Spec.NewHeader = NewHeader;
   Spec.OldCC = OldCC;
Index: clang-tools-extra/trunk/clang-move/ClangMove.cpp
===
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp
@@ -311,10 +311,8 @@
 }
 
 void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  SmallVector ClassNames;
-  llvm::StringRef(Spec.Names).split(ClassNames, ',');
   Optional InMovedClassNames;
-  for (StringRef ClassName : ClassNames) {
+  for (StringRef ClassName : Spec.Names) {
 llvm::StringRef GlobalClassName = ClassName.trim().ltrim(':');
 const auto HasName = hasName(("::" + GlobalClassName).str());
 InMovedClassNames =
Index: clang-tools-extra/trunk/clang-move/ClangMove.h
===
--- clang-tools-extra/trunk/clang-move/ClangMove.h
+++ clang-tools-extra/trunk/clang-move/ClangMove.h
@@ -37,9 +37,8 @@
   };
 
   struct MoveDefinitionSpec {
-// A comma-separated list of fully qualified names, e.g. "Foo",
-// "a::Foo, b::Foo".
-std::string Names;
+// The list of fully qualified names, e.g. Foo, a::Foo, b::Foo.
+SmallVector 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-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
===
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
@@ -213,7 +213,7 @@
 
 TEST(ClangMove, MoveHeaderAndCC) {
   move::ClangMoveTool::MoveDefinitionSpec Spec;
-  Spec.Names = "a::b::Foo";
+  Spec.Names = { "a::b::Foo" };
   

[PATCH] D25586: [clang-move] Use cl::list and cl::CommaSeparated for the list of names.

[Haojian Wu via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


Repository:
  rL LLVM

https://reviews.llvm.org/D25586



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25586: [clang-move] Use cl::list and cl::CommaSeparated for the list of names.

[Alexander Shaposhnikov via cfe-commits]

alexshap created this revision.
alexshap added reviewers: ioeric, klimek.
alexshap added a subscriber: cfe-commits.
alexshap set the repository for this revision to rL LLVM.

This diff replaces manual parsing of the comma-separated list of names with 
cl::list and cl::CommaSeparated.

Test plan: make -j8 check-clang-tools


Repository:
  rL LLVM

https://reviews.llvm.org/D25586

Files:
  clang-move/ClangMove.cpp
  clang-move/ClangMove.h
  clang-move/tool/ClangMoveMain.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.Names = "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.Names = "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.Names = "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.Names = "NonExistFoo";
+  Spec.Names = { "NonExistFoo" };
   Spec.OldHeader = "foo.h";
   Spec.OldCC = "foo.cc";
   Spec.NewHeader = "new_foo.h";
Index: clang-move/tool/ClangMoveMain.cpp
===
--- clang-move/tool/ClangMoveMain.cpp
+++ clang-move/tool/ClangMoveMain.cpp
@@ -38,10 +38,10 @@
 
 cl::OptionCategory ClangMoveCategory("clang-move options");
 
-cl::opt
-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::list Names("names", cl::CommaSeparated, cl::OneOrMore,
+cl::desc("The list of the names of classes being "
+ "moved, e.g. \"Foo,a::Foo,b::Foo\"."),
+cl::cat(ClangMoveCategory));
 
 cl::opt
 OldHeader("old_header",
@@ -90,7 +90,8 @@
   tooling::RefactoringTool Tool(OptionsParser.getCompilations(),
 OptionsParser.getSourcePathList());
   move::ClangMoveTool::MoveDefinitionSpec Spec;
-  Spec.Names = Names;
+  Spec.Names = { Names.begin(), Names.end() };
+  
   Spec.OldHeader = OldHeader;
   Spec.NewHeader = NewHeader;
   Spec.OldCC = OldCC;
Index: clang-move/ClangMove.h
===
--- clang-move/ClangMove.h
+++ clang-move/ClangMove.h
@@ -37,9 +37,8 @@
   };
 
   struct MoveDefinitionSpec {
-// A comma-separated list of fully qualified names, e.g. "Foo",
-// "a::Foo, b::Foo".
-std::string Names;
+// The list of fully qualified names, e.g. Foo, a::Foo, b::Foo.
+llvm::SmallVector 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
@@ -328,10 +328,8 @@
 }
 
 void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  SmallVector ClassNames;
-  llvm::StringRef(Spec.Names).split(ClassNames, ',');
   Optional InMovedClassNames;
-  for (StringRef ClassName : ClassNames) {
+  for (StringRef ClassName : Spec.Names) {
 llvm::StringRef GlobalClassName = ClassName.trim().ltrim(':');
 const auto HasName = hasName(("::" + GlobalClassName).str());
 InMovedClassNames =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits