[PATCH] D24380: [migrate-tool] Framework for a codebase-dependent migration tool.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 16 06:10:10 PDT 2016
hokein added a comment.
Sorry for the delay.
The patch only contains an unittest for `HeaderGenerato`r, which is not quite enough. Should we create a fake migrate-tool binary to illustrate APIs usage?
================
Comment at: migrate-tool/HeaderGenerator.h:22
@@ +21,3 @@
+public:
+ HeaderGenerator(llvm::StringRef HeaderName);
+
----------------
explicit.
================
Comment at: migrate-tool/MigrateTool.cpp:99
@@ +98,3 @@
+ MovedSymbols.push_back(Spec.getOldName());
+ // FIXME: consider source files. Need to determine whether source files send
+ // with ".cpp" or ".cc" etc.
----------------
s/send/end
================
Comment at: migrate-tool/MigrateTool.h:50
@@ +49,3 @@
+ public:
+ enum class MigrateType {
+ Class,
----------------
What is the 'MigrateType' used for? I can't find any usage in the patch.
================
Comment at: migrate-tool/RefactoringManager.h:28
@@ +27,3 @@
+ addIncludesToFiles(const std::set<llvm::StringRef> &Includes,
+ llvm::ArrayRef<std::string> Files) = 0;
+
----------------
The arguments `llvm::StringRef` and `std::string` look inconsistent to me at first glance. But currently it is fine, we can modify it if needed in the future.
https://reviews.llvm.org/D24380
More information about the cfe-commits
mailing list