Write transform results to disk only once

    
Instead of writing the result of each transform to disk for every
transform, write the results to buffers in memory and pass those buffers
to the next transform as input. Only write the buffers to disk if the
final syntax check passes.

Reviewers: klimek
llvm-svn: 172657

7 files changed, 178 insertions, 31 deletions

diff --git a/clang-tools-extra/cpp11-migrate/CMakeLists.txt b/clang-tools-extra/cpp11-migrate/CMakeLists.txt
index c75447bd78c..d8e2d4b30d8 100644
--- a/clang-tools-extra/cpp11-migrate/CMakeLists.txt
+++ b/clang-tools-extra/cpp11-migrate/CMakeLists.txt
@@ -5,6 +5,7 @@ include_directories(${CMAKE_CURRENT_SOURCE_DIR})
 set (Cpp11MigrateSources
   Cpp11Migrate.cpp
   Transforms.cpp
+  Transform.cpp
   )
 
 # For each transform subdirectory.
diff --git a/clang-tools-extra/cpp11-migrate/Cpp11Migrate.cpp b/clang-tools-extra/cpp11-migrate/Cpp11Migrate.cpp
index 08ae3e0dfbb..4f44f1dcd4f 100644
--- a/clang-tools-extra/cpp11-migrate/Cpp11Migrate.cpp
+++ b/clang-tools-extra/cpp11-migrate/Cpp11Migrate.cpp
@@ -27,27 +27,26 @@
 ///
 //===----------------------------------------------------------------------===//
 
+#include "Transforms.h"
+#include "Transform.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
-#include "Transforms.h"
-#include "Transform.h"
 
 namespace cl = llvm::cl;
 using namespace clang::tooling;
 
-static cl::opt<RiskLevel>
-MaxRiskLevel("risk", cl::desc("Select a maximum risk level:"),
-             cl::values(
-               clEnumValN(RL_Safe, "safe", "Only safe transformations"),
-                 clEnumValN(RL_Reasonable, "reasonable",
-                           "Enable transformations that might change "
-                           "semantics (default)"),
-                 clEnumValN(RL_Risky, "risky",
-                           "Enable transformations that are likely to "
-                           "change semantics"),
+static cl::opt<RiskLevel> MaxRiskLevel(
+    "risk", cl::desc("Select a maximum risk level:"),
+    cl::values(clEnumValN(RL_Safe, "safe", "Only safe transformations"),
+               clEnumValN(RL_Reasonable, "reasonable",
+                          "Enable transformations that might change "
+                          "semantics (default)"),
+               clEnumValN(RL_Risky, "risky",
+                          "Enable transformations that are likely to "
+                          "change semantics"),
                clEnumValEnd),
-             cl::init(RL_Reasonable));
+    cl::init(RL_Reasonable));
 
 int main(int argc, const char **argv) {
   Transforms TransformManager;
@@ -74,23 +73,53 @@ int main(int argc, const char **argv) {
     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(),
-       E = TransformManager.end(); I != E; ++I) {
-    if ((*I)->apply(MaxRiskLevel, OptionsParser.getCompilations(),
-                    OptionsParser.getSourcePathList()) != 0) {
+                                  E = TransformManager.end();
+       I != E; ++I) {
+    if ((*I)->apply(*InputFileStates, MaxRiskLevel,
+                    OptionsParser.getCompilations(),
+                    OptionsParser.getSourcePathList(), *OutputFileStates) !=
+        0) {
+      // FIXME: Improve ClangTool to not abort if just one file fails.
       return 1;
     }
+    std::swap(InputFileStates, OutputFileStates);
+    OutputFileStates->clear();
   }
 
+  // Final state of files is pointed at by InputFileStates.
+
   // Final Syntax check.
   ClangTool EndSyntaxTool(OptionsParser.getCompilations(),
                           OptionsParser.getSourcePathList());
-  if (EndSyntaxTool.run(
-        newFrontendActionFactory<clang::SyntaxOnlyAction>()) != 0) {
-    // FIXME: Revert changes made to files that fail the syntax test.
+  for (FileContentsByPath::const_iterator I = InputFileStates->begin(),
+                                          E = InputFileStates->end();
+       I != E; ++I) {
+    EndSyntaxTool.mapVirtualFile(I->first, I->second);
+  }
+
+  if (EndSyntaxTool.run(newFrontendActionFactory<clang::SyntaxOnlyAction>()) !=
+      0) {
     return 1;
   }
 
+  // Syntax check passed, write results to file.
+  for (FileContentsByPath::const_iterator I = InputFileStates->begin(),
+                                          E = InputFileStates->end();
+       I != E; ++I) {
+    std::string ErrorInfo;
+    llvm::raw_fd_ostream FileStream(I->first.c_str(), ErrorInfo,
+                                    llvm::raw_fd_ostream::F_Binary);
+    FileStream << I->second;
+  }
+
   return 0;
 }
diff --git a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.cpp b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.cpp
index 41337376780..b0e746e03b3 100644
--- a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.cpp
+++ b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.cpp
@@ -19,15 +19,25 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
+#include "clang/Rewrite/Core/Rewriter.h"
 
 using clang::ast_matchers::MatchFinder;
 using namespace clang::tooling;
 using namespace clang;
 
-int LoopConvertTransform::apply(RiskLevel MaxRisk,
+int LoopConvertTransform::apply(const FileContentsByPath &InputStates,
+                                RiskLevel MaxRisk,
                                 const CompilationDatabase &Database,
-                                const std::vector<std::string> &SourcePaths) {
+                                const std::vector<std::string> &SourcePaths,
+                                FileContentsByPath &ResultStates) {
   RefactoringTool LoopTool(Database, SourcePaths);
+
+  for (FileContentsByPath::const_iterator I = InputStates.begin(),
+       E = InputStates.end();
+       I != E; ++I) {
+    LoopTool.mapVirtualFile(I->first, I->second);
+  }
+
   StmtAncestorASTVisitor ParentFinder;
   StmtGeneratedVarNameMap GeneratedDecls;
   ReplacedVarsMap ReplacedVars;
@@ -53,11 +63,19 @@ int LoopConvertTransform::apply(RiskLevel MaxRisk,
                                   &RejectedChanges,
                                   MaxRisk, LFK_PseudoArray);
   Finder.addMatcher(makePseudoArrayLoopMatcher(), &PseudoarrrayLoopFixer);
-  if (int result = LoopTool.runAndSave(newFrontendActionFactory(&Finder))) {
+
+  if (int result = LoopTool.run(newFrontendActionFactory(&Finder))) {
     llvm::errs() << "Error encountered during translation.\n";
     return result;
   }
 
+  RewriterContainer Rewrite(LoopTool.getFiles());
+
+  // FIXME: Do something if some replacements didn't get applied?
+  LoopTool.applyAllReplacements(Rewrite.getRewriter());
+
+  collectResults(Rewrite.getRewriter(), ResultStates);
+
   if (AcceptedChanges > 0) {
     setChangesMade();
   }
diff --git a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.h b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.h
index ede1de9e062..850d64990fb 100644
--- a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.h
+++ b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.h
@@ -17,13 +17,19 @@
 #define LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_LOOP_CONVERT_H
 
 #include "Transform.h"
+#include "llvm/Support/Compiler.h" // For LLVM_OVERRIDE
 
+/// \brief Subclass of Transform that transforms for-loops into range-based
+/// for-loops where possible.
 class LoopConvertTransform : public Transform {
 public:
 
-  virtual int apply(RiskLevel MaxRiskLEvel,
+  /// \brief \see Transform::run().
+  virtual int apply(const FileContentsByPath &InputStates,
+                    RiskLevel MaxRiskLevel,
                     const clang::tooling::CompilationDatabase &Database,
-                    const std::vector<std::string> &SourcePaths) LLVM_OVERRIDE;
+                    const std::vector<std::string> &SourcePaths,
+                    FileContentsByPath &ResultStates) LLVM_OVERRIDE;
 };
 
 #endif // LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_LOOP_CONVERT_H
diff --git a/clang-tools-extra/cpp11-migrate/Makefile b/clang-tools-extra/cpp11-migrate/Makefile
index db7835fcb53..576223d846d 100644
--- a/clang-tools-extra/cpp11-migrate/Makefile
+++ b/clang-tools-extra/cpp11-migrate/Makefile
@@ -17,7 +17,7 @@ TOOL_NO_EXPORTS = 1
 
 include $(CLANG_LEVEL)/../../Makefile.config
 
-SOURCES = Cpp11Migrate.cpp Transforms.cpp
+SOURCES = Cpp11Migrate.cpp Transforms.cpp Transform.cpp
 
 # For each Transform subdirectory add to SOURCES and BUILT_SOURCES.
 # BUILT_SOURCES ensures a subdirectory is created to house object files from
diff --git a/clang-tools-extra/cpp11-migrate/Transform.cpp b/clang-tools-extra/cpp11-migrate/Transform.cpp
new file mode 100644
index 00000000000..c3203793fe8
--- /dev/null
+++ b/clang-tools-extra/cpp11-migrate/Transform.cpp
@@ -0,0 +1,35 @@
+#include "Transform.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang;
+
+void collectResults(clang::Rewriter &Rewrite,
+                    FileContentsByPath &Results) {
+  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();
+
+    // Get a copy of the rewritten buffer from the Rewriter.
+    llvm::raw_string_ostream StringStream(ResultEntry.second);
+    I->second.write(StringStream);
+
+    // Cause results to be written to ResultEntry.second.
+    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);
+  }
+}
diff --git a/clang-tools-extra/cpp11-migrate/Transform.h b/clang-tools-extra/cpp11-migrate/Transform.h
index b730f9f1265..7c0861275b8 100644
--- a/clang-tools-extra/cpp11-migrate/Transform.h
+++ b/clang-tools-extra/cpp11-migrate/Transform.h
@@ -15,8 +15,18 @@
 #ifndef LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_TRANSFORM_H
 #define LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_TRANSFORM_H
 
-#include "clang/Tooling/CompilationDatabase.h"
 #include <vector>
+#include <string>
+
+// For RewriterContainer
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/Support/raw_ostream.h"
+////
+
 
 /// \brief Description of the riskiness of actions that can be taken by
 /// transforms.
@@ -31,13 +41,56 @@ enum RiskLevel {
   RL_Risky
 };
 
-// Forward Declarations
+// Forward declarations
 namespace clang {
 namespace tooling {
 class CompilationDatabase;
 } // namespace tooling
 } // namespace clang
 
+/// \brief First item in the pair is the path of a file and the second is a
+/// buffer with the possibly modified contents of that file.
+typedef std::vector<std::pair<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);
+
+/// \brief Class for containing a Rewriter instance and all of
+/// its lifetime dependencies.
+///
+/// Subclasses of Transform using RefactoringTools will need to create
+/// Rewriters in order to apply Replacements and get the resulting buffer.
+/// Rewriter requires some objects to exist at least as long as it does so this
+/// class contains instances of those objects.
+///
+/// FIXME: These objects should really come from somewhere more global instead
+/// of being recreated for every Transform subclass, especially diagnostics.
+class RewriterContainer {
+public:
+  RewriterContainer(clang::FileManager &Files)
+    : DiagOpts(new clang::DiagnosticOptions()),
+      DiagnosticPrinter(llvm::errs(), DiagOpts.getPtr()),
+      Diagnostics(llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs>(
+                    new clang::DiagnosticIDs()),
+                  DiagOpts.getPtr(), &DiagnosticPrinter, false),
+      Sources(Diagnostics, Files),
+      Rewrite(Sources, DefaultLangOptions) {}
+
+  clang::Rewriter &getRewriter() { return Rewrite; }
+
+private:
+  clang::LangOptions DefaultLangOptions;
+  llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> DiagOpts;
+  clang::TextDiagnosticPrinter DiagnosticPrinter;
+  clang::DiagnosticsEngine Diagnostics;
+  clang::SourceManager Sources;
+  clang::Rewriter Rewrite;
+};
+
 /// \brief Abstract base class for all C++11 migration transforms.
 class Transform {
 public:
@@ -49,11 +102,16 @@ public:
 
   /// \brief Apply a transform to all files listed in \p SourcePaths.
   ///
-  /// \p Database must contain information for how to compile all files in
-  /// \p SourcePaths.
-  virtual int apply(RiskLevel MaxRiskLevel,
+  /// \p Database must contain information for how to compile all files in \p
+  /// SourcePaths. \p InputStates contains the file contents of files in \p
+  /// SourcePaths and should take precedence over content of files on disk.
+  /// Upon return, \p ResultStates shall contain the result of performing this
+  /// transform on the files listed in \p SourcePaths.
+  virtual int apply(const FileContentsByPath &InputStates,
+                    RiskLevel MaxRiskLevel,
                     const clang::tooling::CompilationDatabase &Database,
-                    const std::vector<std::string> &SourcePaths) = 0;
+                    const std::vector<std::string> &SourcePaths,
+                    FileContentsByPath &ResultStates) = 0;
 
   /// \brief Query if changes were made during the last call to apply().
   bool getChangesMade() const { return ChangesMade; }