[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