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

[Tareq A. Siraj]

================
Comment at: cpp11-migrate/Core/FileOverrides.h:63
@@ +62,3 @@
+  /// \brief getter for TransformReplacementsDoc.
+  TransformDocument &getTransformReplacementsDoc() {
+    return TransformReplacementsDoc;
----------------
Edwin Vane wrote:
> I'm not sure how you feel about this but here's a suggestion. To try and maintain semantics, how about we get rid of the non-const iterator providers in FileOverrides, mark this member function `const` and use this impl instead:
> 
>   return const_cast<HeaderOverride*>(this)->TransformReplacementsDoc
> 
> I think it's safe here because we know the returned ref is only used for writing out to file and thus won't be modified.
Fixed in upcoming patch.

================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:99
@@ +98,3 @@
+       I != E; ++I) {
+    const Replacement &R = *I;
+    if (R.getFilePath() == MainFileName)
----------------
Edwin Vane wrote:
> I'm not sure this temp variable really helps readability at all. I'd just use *I and I-> below.
Fixed in upcoming patch.

================
Comment at: cpp11-migrate/Core/Replacement.h:1
@@ +1,2 @@
+//===-- Core/Replacement.h --------------------------------------*- C++ -*-===//
+//
----------------
Edwin Vane wrote:
> Since this file contains stuff only used for YAML, can it be merged with ReplacementsYaml.h?
Merged.

================
Comment at: test/cpp11-migrate/HeaderReplacements/common.cpp:3
@@ +2,3 @@
+// in main.cpp
+// RUN: ls
+
----------------
Edwin Vane wrote:
> Looking at lit source it looks like you can just say `// END.`
Fixed.

================
Comment at: test/cpp11-migrate/HeaderReplacements/main.cpp:6
@@ +5,3 @@
+// RUN: mkdir -p %t/Test
+// RUN: cp %p/main.cpp %p/common.cpp %p/common.h %t/Test
+// RUN: cpp11-migrate -loop-convert -headers -include=%t/Test %t/Test/main.cpp %t/Test/common.cpp --
----------------
Edwin Vane wrote:
> Use %S instead of %p for consistency. We have other tests that use it already.
Fixed.

================
Comment at: test/cpp11-migrate/HeaderReplacements/main.cpp:8
@@ +7,3 @@
+// RUN: cpp11-migrate -loop-convert -headers -include=%t/Test %t/Test/main.cpp %t/Test/common.cpp --
+// RUN: ls %t/Test/main.cpp_common.h_*.yaml | wc -l | grep 1
+// RUN: ls %t/Test/common.cpp_common.h_*.yaml | wc -l | grep 1
----------------
Edwin Vane wrote:
> Could you add a comment explaning what these commands are doing? Especially the seds.
Done.

================
Comment at: cpp11-migrate/Core/ReplacementsYaml.h:34
@@ +33,3 @@
+    unsigned int Length = R.getLength();
+    llvm::StringRef ReplacementText = R.getReplacementText();
+    Io.mapRequired("Offset", Offset);
----------------
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?


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