[PATCH] cpp11-migrate: FileOverrides/Transform refactoring.

Tareq A. Siraj tareq.a.siraj at intel.com
Wed Jul 10 12:11:39 PDT 2013



================
Comment at: cpp11-migrate/Core/FileOverrides.h:102
@@ +101,3 @@
+
+public:
+  FileOverrides() {}
----------------
Already under public:

================
Comment at: cpp11-migrate/Core/FileOverrides.h:87
@@ +86,3 @@
+
+private:
+  const std::string MainFileName;
----------------
Already under private:

================
Comment at: cpp11-migrate/Core/FileOverrides.h:88
@@ -62,1 +87,3 @@
+private:
+  const std::string MainFileName;
   std::string MainFileOverride;
----------------
Do we need to store MainFileName? Can we use a StringRef instead.

================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:19
@@ -18,2 +18,3 @@
 #include "clang/Basic/SourceManager.h"
-#include "llvm/Support/Path.h"
+
+#include "clang/Format/Format.h"
----------------
Unnecessary empty line.

================
Comment at: cpp11-migrate/Core/SyntaxCheck.h:19
@@ -18,4 +18,3 @@
 
-#include "Core/FileOverrides.h"
-
+#include <string>
 #include <vector>
----------------
Do we need this?

================
Comment at: cpp11-migrate/Core/SyntaxCheck.cpp:17
@@ +16,3 @@
+#include "Core/SyntaxCheck.h"
+
+#include "Core/FileOverrides.h"
----------------
Unnecessary empty line.

================
Comment at: cpp11-migrate/Core/Transform.h:53
@@ -53,3 +52,3 @@
 
-class RewriterManager;
+class FileOverrides;
 
----------------
Do we need this?

================
Comment at: cpp11-migrate/Core/Transform.cpp:17
@@ -16,1 +16,3 @@
 #include "Core/Transform.h"
+
+#include "Core/FileOverrides.h"
----------------
Unnecessary empty line.

================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:19
@@ -18,3 +20,2 @@
 #include "Core/SyntaxCheck.h"
-#include "Core/Transforms.h"
 #include "Core/Transform.h"
----------------
No need to re-arrange headers if not needed.

================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:35
@@ -32,2 +34,3 @@
 namespace cl = llvm::cl;
+using namespace clang;
 using namespace clang::tooling;
----------------
Do we need this?

================
Comment at: unittests/cpp11-migrate/TransformTest.cpp:14
@@ +13,3 @@
+
+#include "Core/FileOverrides.h"
+
----------------
Do we need this for this test?


http://llvm-reviews.chandlerc.com/D1122



More information about the cfe-commits mailing list