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

Edwin Vane edwin.vane at intel.com
Thu Jul 25 12:17:09 PDT 2013



================
Comment at: cpp11-migrate/Core/FileOverrides.h:99
@@ +98,3 @@
+
+  /// \brief Adds the transform id and replacement to the
+  /// TransformReplacementsDoc.
----------------
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.

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

================
Comment at: cpp11-migrate/Core/FileOverrides.h:77
@@ +76,3 @@
+  }
+  void setFileName(const llvm::StringRef S) {
+    TransformReplacementsDoc.FileName = S;
----------------
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.

================
Comment at: cpp11-migrate/Core/FileOverrides.h:101
@@ +100,3 @@
+  /// TransformReplacementsDoc.
+  void addTransformReplacements(llvm::StringRef TransformID,
+                                const clang::tooling::Replacements &Replaces);
----------------
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.

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

================
Comment at: unittests/cpp11-migrate/ReplacementsYamlTest.cpp:14
@@ +13,3 @@
+
+#include "Utility.h"
+#include "Core/FileOverrides.h"
----------------
No longer necessary. Looks like there are some other unnecessary #includes now too.


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

BRANCH
  write_replacements_to_disk

ARCANIST PROJECT
  clang-tools-extra



More information about the cfe-commits mailing list