[cfe-commits] [PATCH] Write transform results to disk only once

Manuel Klimek klimek at google.com
Wed Jan 16 05:11:11 PST 2013


  Generally LG, just some nits.


================
Comment at: cpp11-migrate/Transform.h:51
@@ -40,1 +50,3 @@
 
+typedef std::vector<std::pair<std::string, std::string> > RefactoringResults;
+
----------------
A comment what the pairs mean would be really helpful :)

We could also call this FileContentsByPath or something, which would help me greatly to understand the meaning from the name.

================
Comment at: cpp11-migrate/LoopConvert/LoopConvert.cpp:28
@@ -26,2 +27,3 @@
 
-int LoopConvertTransform::apply(RiskLevel MaxRisk,
+int LoopConvertTransform::apply(const RefactoringResults &InputStates,
+                                RiskLevel MaxRisk,
----------------
Perhaps s/InputStates/OverwrittenFileContents/ and s/ResultStates/ChangedFileContents/?

================
Comment at: cpp11-migrate/Transform.cpp:10
@@ +9,3 @@
+void collectResults(RefactoringResults &Results,
+                    clang::Rewriter &Rewrite) {
+  for (Rewriter::buffer_iterator I = Rewrite.buffer_begin(),
----------------
Personally, I'd put the output parameter last...

================
Comment at: cpp11-migrate/LoopConvert/LoopConvert.cpp:72
@@ -60,1 +71,3 @@
 
+  OwningPtr<RewriterContainer>
+    Rewrite(new RewriterContainer(LoopTool.getFiles()));
----------------
Any reason to not just do:
RewriterContainer Rewrite(LoopTool.getFiles());
?


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



More information about the cfe-commits mailing list