[PATCH] Adding Replacement serialization support

[Daniel Jasper]

================
Comment at: unittests/Tooling/ReplacementsYamlTest.cpp:31
@@ +30,3 @@
+
+TEST(ReplacementsYamlTest, writeReadTest) {
+
----------------
Most importantly: I think it is a bad practice to solely have a round-trip test. You could easily mix up two parameters (say offset and length) and the round-trip test would still pass (just the YAML files wouldn't make sense). Could we extend the tests below to verify the contents of the generated file and the parsed document, respectively?

nit: The second part of such a definition usually:
- Does not include "test"
- Starts upper case
- Is written as a third person verb (although this is not really consistent)
Also, "writeRead" does not really mean anything to me. How about something along the lines of "SuccessfullyRoundTrips"?


================
Comment at: unittests/Tooling/ReplacementsYamlTest.cpp:100
@@ +99,3 @@
+  ASSERT_NE(YamlContent.length(), 0u);
+  ASSERT_EQ(std::string::npos, YamlContent.find("Context:"));
+}
----------------
Would it be hard to actually verify the entire contents?

================
Comment at: unittests/Tooling/ReplacementsYamlTest.cpp:115
@@ +114,3 @@
+  YAML >> DocActual;
+  ASSERT_NO_ERROR(YAML.error());
+}
----------------
(see first comment): Verify the contents of the parsed document.

================
Comment at: include/clang/Tooling/ReplacementsYaml.h:28
@@ +27,3 @@
+/// \brief The top-level YAML document that contains all Replacements.
+struct ReplacementsDocument {
+  /// A freeform chunk of text to describe the context of the replacements in
----------------
I also think that this should not be called document, but for a very different reason:

I think this is more generic and describes a "group" or "unit" (not saying that that would be a good name) of replacements. We need that for other things, e.g. to apply a group of replacements atomically if some of the replacements have conflicts with other changes.

Thus, I'd rename this (to open bikeshedding: ReplacementGroup) and move it to Refactoring.h.


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