[PATCH] Fix for when transforms are lost

[Jack Yang]

jy7.yang added you to the CC list for the revision "Fix for when transforms are lost".

Hi klimek,

In cpp11-migrate, when multiple transforms are requested, if the second
transformation does not change anything, the previous transforms are lost.

This happens because collectResults() in Transform.cpp only pushes onto the
Results vector when the Rewriter's buffer is not empty. When no transforms are
needed for a particular file, the Rewriter is empty, and thus the contents of
that file will not appear in the Results vector.

To address this, instead of adding the file contents when a transform is
applied, a copy of the data structure is made at the start of collectResults()
so that the file contents of all previously modified files are preserved.

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

Files:
  cpp11-migrate/Cpp11Migrate.cpp
  cpp11-migrate/LoopConvert/LoopConvert.cpp
  cpp11-migrate/Transform.cpp
  cpp11-migrate/Transform.h
  cpp11-migrate/UseNullptr/UseNullptr.cpp

Index: cpp11-migrate/Cpp11Migrate.cpp
===================================================================
--- cpp11-migrate/Cpp11Migrate.cpp
+++ cpp11-migrate/Cpp11Migrate.cpp
@@ -84,12 +84,8 @@
     return 1;
   }
 
-  unsigned int NumFiles = OptionsParser.getSourcePathList().size();
-
   FileContentsByPath FileStates1, FileStates2,
       *InputFileStates = &FileStates1, *OutputFileStates = &FileStates2;
-  FileStates1.reserve(NumFiles);
-  FileStates2.reserve(NumFiles);
 
   // Apply transforms.
   for (Transforms::const_iterator I = TransformManager.begin(),
Index: cpp11-migrate/LoopConvert/LoopConvert.cpp
===================================================================
--- cpp11-migrate/LoopConvert/LoopConvert.cpp
+++ cpp11-migrate/LoopConvert/LoopConvert.cpp
@@ -74,7 +74,7 @@
   // FIXME: Do something if some replacements didn't get applied?
   LoopTool.applyAllReplacements(Rewrite.getRewriter());
 
-  collectResults(Rewrite.getRewriter(), ResultStates);
+  collectResults(Rewrite.getRewriter(), InputStates, ResultStates);
 
   if (AcceptedChanges > 0) {
     setChangesMade();
Index: cpp11-migrate/Transform.cpp
===================================================================
--- cpp11-migrate/Transform.cpp
+++ cpp11-migrate/Transform.cpp
@@ -7,29 +7,31 @@
 using namespace clang;
 
 void collectResults(clang::Rewriter &Rewrite,
+                    const FileContentsByPath &InputStates,
                     FileContentsByPath &Results) {
+  // Copy the contents of InputStates to be modified.
+  Results = InputStates;
+
   for (Rewriter::buffer_iterator I = Rewrite.buffer_begin(),
                                  E = Rewrite.buffer_end();
        I != E; ++I) {
     const FileEntry *Entry = Rewrite.getSourceMgr().getFileEntryForID(I->first);
     assert(Entry != 0 && "Expected a FileEntry");
     assert(Entry->getName() != 0 &&
            "Unexpected NULL return from FileEntry::getName()");
 
-    FileContentsByPath::value_type ResultEntry;
-
-    ResultEntry.first = Entry->getName();
+    std::string ResultBuf;
 
     // Get a copy of the rewritten buffer from the Rewriter.
-    llvm::raw_string_ostream StringStream(ResultEntry.second);
+    llvm::raw_string_ostream StringStream(ResultBuf);
     I->second.write(StringStream);
 
-    // Cause results to be written to ResultEntry.second.
+    // Cause results to be written to ResultBuf.
     StringStream.str();
 
     // FIXME: Use move semantics to avoid copies of the buffer contents if
     // benchmarking shows the copies are expensive, especially for large source
     // files.
-    Results.push_back(ResultEntry);
+    Results[Entry->getName()] = ResultBuf;
   }
 }
Index: cpp11-migrate/Transform.h
===================================================================
--- cpp11-migrate/Transform.h
+++ cpp11-migrate/Transform.h
@@ -48,16 +48,18 @@
 } // namespace tooling
 } // namespace clang
 
-/// \brief First item in the pair is the path of a file and the second is a
+/// \brief The key is the path of a file, which is mapped to a
 /// buffer with the possibly modified contents of that file.
-typedef std::vector<std::pair<std::string, std::string> > FileContentsByPath;
+typedef std::map<std::string, std::string> FileContentsByPath;
 
 /// \brief In \p Results place copies of the buffers resulting from applying
 /// all rewrites represented by \p Rewrite.
 ///
 /// \p Results is made up of pairs {filename, buffer contents}. Pairs are
 /// simply appended to \p Results.
-void collectResults(clang::Rewriter &Rewrite, FileContentsByPath &Results);
+void collectResults(clang::Rewriter &Rewrite,
+                    const FileContentsByPath &InputStates,
+                    FileContentsByPath &Results);
 
 /// \brief Class for containing a Rewriter instance and all of
 /// its lifetime dependencies.
Index: cpp11-migrate/UseNullptr/UseNullptr.cpp
===================================================================
--- cpp11-migrate/UseNullptr/UseNullptr.cpp
+++ cpp11-migrate/UseNullptr/UseNullptr.cpp
@@ -58,7 +58,7 @@
   // FIXME: Do something if some replacements didn't get applied?
   UseNullptrTool.applyAllReplacements(Rewrite.getRewriter());
 
-  collectResults(Rewrite.getRewriter(), ResultStates);
+  collectResults(Rewrite.getRewriter(), InputStates, ResultStates);
 
   if (AcceptedChanges > 0) {
     setChangesMade();
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D410.1.patch
Type: text/x-patch
Size: 4359 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130214/d869c869/attachment.bin>