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

[Edwin Vane]

================
Comment at: cpp11-migrate/Core/ReplacementsYaml.h:43
@@ +42,3 @@
+
+// ScalarTraits to read/write std::string objects.
+template <>
----------------
Doxygen-style comments.

================
Comment at: cpp11-migrate/Core/ReplacementsYaml.h:66
@@ +65,3 @@
+    Io.mapRequired("Length", Length);
+    Io.mapRequired("ReplacementText", ReplacementText);
+  }
----------------
I think the use of temp variables here will break YAML file **reading**. the mapRequired() is forming a map between the key "X" and the address of a variable. If you're providing a temp var, whatever it reads from the file will be stored in some random location in memory and never make it to the destination structure. 

It occurs to me that since you're using an existing structure we can't really change the interface of (tooling::Replacement) you might have to use [[http://llvm.org/docs/YamlIO.html#normalization|normalization]].

I suggest you add a YAML file-reading unit test. All you have to do is ensure the file parses and the resulting doc contains expected info.

================
Comment at: cpp11-migrate/Core/ReplacementsYaml.h:34
@@ +33,3 @@
+    unsigned int Length = R.getLength();
+    llvm::StringRef ReplacementText = R.getReplacementText();
+    Io.mapRequired("Offset", Offset);
----------------
Tareq A. Siraj wrote:
> Edwin Vane wrote:
> > I discovered this doesn't work for reading. Check out `ScalarTraits<std::string>` in https://github.com/revane/migmerge_git/blob/master/YAML.h.
> Did you mean to refactor the tooling::Replacement class to return a string instead or just changing this to std::string here?
See my comment regarding normalization.


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

BRANCH
  write_replacements_to_disk

ARCANIST PROJECT
  clang-tools-extra