[PATCH] cpp11-migrate: Write header replacements to disk

Edwin Vane edwin.vane at intel.com
Thu Jul 25 08:15:39 PDT 2013

Comment at: cpp11-migrate/Core/FileOverrides.h:101
@@ +100,3 @@
+  /// TransformReplacementsDoc.
+  void addTransformReplacement(llvm::StringRef TransformID,
+                               const clang::tooling::Replacement &R) {
This will result in a whole bunch of TransformReplacements with only a single Replacment will it not? There should be some sort of lookup by transform ID and the replacement should be **added** to that. Since replacements are provided in a batch a whole transform at a time you should just accept the entire replacements set here instead of one replacement at a time.

Comment at: cpp11-migrate/Core/ReplacementsYaml.h:69
@@ +68,3 @@
+          Length(R.getLength()) {
+      // We need to put quotes around the string to make this a YAML string.
+      ReplacementText = std::string("\"") +
This should really happen in ScalarTraits<std::string>::output().

Comment at: unittests/cpp11-migrate/ReplacementsYamlTest.cpp:29
@@ +28,3 @@
+  ReplacementsYamlTest() {
+    ASSERT_NO_ERROR(sys::fs::current_path(SourceFileName));
+    ASSERT_NO_ERROR(sys::fs::current_path(HeaderFileName));
Don't write to and read from files on disk. This is just asking for trouble. We have no guarantee what the "current_path" will even be and risk polluting whatever directory structure this test runs in. YAML doesn't require files to write to so create a raw_string_ostream instead and write to blocks in memory.




More information about the cfe-commits mailing list