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

[Manuel Klimek]

================
Comment at: cpp11-migrate/Cpp11Migrate.cpp:115
@@ +114,3 @@
+  // Syntax check passed, write results to file.
+  for (RefactoringResults::const_iterator I = InputFileStates->begin(),
+       E = InputFileStates->end();
----------------
You might want to consider using clang-format to indent this :)

================
Comment at: cpp11-migrate/RefactoringResults.h:34
@@ +33,3 @@
+public:
+  typedef std::pair<char*, std::string> ResultEntry;
+  typedef std::vector<ResultEntry> ResultVec;
----------------
why not pair<string, string>?

================
Comment at: cpp11-migrate/RefactoringResults.h:35
@@ +34,3 @@
+  typedef std::pair<char*, std::string> ResultEntry;
+  typedef std::vector<ResultEntry> ResultVec;
+  typedef ResultVec::const_iterator const_iterator;
----------------
Any reason for this typedef? Seems to me like writing std::vector<ResultEntry> is no harder to write than ResultVec, and it's most certainly easier to read, as I don't have to jump back to see what it defines.

================
Comment at: cpp11-migrate/RefactoringResults.cpp:54
@@ +53,3 @@
+    // This would be a little less wonky using move semantics. The goal is to
+    // avoid potentially expensive copies of ResultEntry.
+    Data.resize(Data.size() + 1);
----------------
This looks very much like a premature optimization. If we don't have a benchmark, I would not make the code more convoluted to "avoid copies".

================
Comment at: cpp11-migrate/RefactoringResults.h:32
@@ +31,3 @@
+/// by this class.
+class RefactoringResults {
+public:
----------------
After reading through all of this, I'm not convinced that we need a class, and can't just use a vector<ResultEntry> and a method
vector<ResultEntry> collectResults(Rewriter&)


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