[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