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

Edwin Vane edwin.vane at intel.com
Wed Jul 10 12:37:58 PDT 2013


  In case I missed some instances, in general, don't add extra blank lines in the block of `#include`s at the top of the file.


================
Comment at: cpp11-migrate/Core/FileOverrides.h:48
@@ -45,2 +47,3 @@
 
 /// \brief Container storing the file content overrides for a source file.
+class SourceOverrides {
----------------
Can you add //...for a source file and any headers included by the source file either directly or indirectly to which changes have been made.//

================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:153
@@ +152,3 @@
+  if (Override == NULL)
+    Override = new SourceOverrides(Filename);
+  return *Override;
----------------
This looks like a memory leak to me. Should not this new SourceOverrides be stored in Overrides?

================
Comment at: cpp11-migrate/Core/SyntaxCheck.cpp:17
@@ +16,3 @@
+#include "Core/SyntaxCheck.h"
+
+#include "Core/FileOverrides.h"
----------------
Tareq A. Siraj wrote:
> Unnecessary empty line.
Unneeded vertical whitespace.

================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:23
@@ -22,1 +22,3 @@
+#include "Core/Transforms.h"
+
 #include "LoopConvert/LoopConvert.h"
----------------
Unnecessary blank line.

================
Comment at: unittests/cpp11-migrate/FileOverridesTest.cpp:43
@@ +42,3 @@
+    ASSERT_NO_ERROR(
+        sys::fs::createTemporaryFile("cpp11-migrate", "cpp", FD, Path));
+
----------------
I recommend not actually creating files on disk. Instead, create files in memory and use SourceManager.overrideFileContents() to make it use the file in memory. This means you need access to the SourceManager created with the rewriter in applyReplacements() so you'll need to think about how to make this work.

================
Comment at: unittests/cpp11-migrate/PerfSupportTest.cpp:1
@@ -1,1 +1,2 @@
-#include "gtest/gtest.h"
+//===- cpp11-migrate/PerfSupportTest.cpp - PerfSupport unit tests --------===//
+//
----------------
Adding headers is unrelated to your refactoring. please put them in another commit.

================
Comment at: unittests/cpp11-migrate/TransformTest.cpp:1
@@ -1,1 +1,2 @@
-#include "gtest/gtest.h"
+//===- cpp11-migrate/TransformTest.cpp - Transform unit tests -------------===//
+//
----------------
Changes to this file are not related to your refactoring. Please put in another commit. And don't add extra blank lines between `#include`s.

================
Comment at: unittests/cpp11-migrate/IncludeExcludeTest.cpp:1
@@ +1,2 @@
+//===- cpp11-migrate/IncludeExcludeTest.cpp - IncludeExclude unit tests ---===//
+//
----------------
Changes to this file are unrelated to your refactoring. Please put in another commit.

================
Comment at: unittests/cpp11-migrate/FileOverridesTest.cpp:11
@@ +10,3 @@
+#include "Core/FileOverrides.h"
+
+#include "gtest/gtest.h"
----------------
Remove extra blank lines from include statements.


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



More information about the cfe-commits mailing list