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

[Tareq A. Siraj]

================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:127
@@ -99,1 +126,3 @@
+  if (TrackChanges)
+    adjustChangedRanges(Replaces);
 }
----------------
Edwin Vane wrote:
> Now you're duplicating the Replacement grouping functionality. Can we just pass the ReplacementsMap through to adjustChangedRanges()?
Fixed.

================
Comment at: cpp11-migrate/Core/FileOverrides.h:84
@@ +83,3 @@
+  /// @{
+  llvm::StringRef getFileOverride() const { return FileOverride; }
+  void setFileOverride(const llvm::StringRef S) { FileOverride = S; }
----------------
Edwin Vane wrote:
> Can we rename these get/setContentOverride().
Fixed.

================
Comment at: cpp11-migrate/Core/FileOverrides.h:77
@@ +76,3 @@
+  }
+  void setFileName(const llvm::StringRef S) {
+    TransformReplacementsDoc.FileName = S;
----------------
Edwin Vane wrote:
> Now that HeaderOverride is a class with a public interface, we should clean up this behaviour. The filename of a HeaderOverride object should not be mutable. Let's make sure the constructor is the only way to set the file name.
Fixed.

================
Comment at: cpp11-migrate/Core/FileOverrides.h:99
@@ +98,3 @@
+
+  /// \brief Adds the transform id and replacement to the
+  /// TransformReplacementsDoc.
----------------
Edwin Vane wrote:
> I think we can document this function without explaining internals. Just say say this function stores the replacements made by a transform to the header this class represents.
Fixed.

================
Comment at: cpp11-migrate/Core/FileOverrides.h:101
@@ +100,3 @@
+  /// TransformReplacementsDoc.
+  void addTransformReplacements(llvm::StringRef TransformID,
+                                const clang::tooling::Replacements &Replaces);
----------------
Edwin Vane wrote:
> How about naming this `recordReplacements()` to make it clear we're storing a copy and this function has nothing to do with the main replacement mechanism provided by SourceOverrides.
Fixed.

================
Comment at: unittests/cpp11-migrate/ReplacementsYamlTest.cpp:14
@@ +13,3 @@
+
+#include "Utility.h"
+#include "Core/FileOverrides.h"
----------------
Edwin Vane wrote:
> No longer necessary. Looks like there are some other unnecessary #includes now too.
Utility.h is needed (line 67). Cleaned up the rest.


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

BRANCH
  write_replacements_to_disk

ARCANIST PROJECT
  clang-tools-extra