cpp11-migrate: Transform now responsible for applying replacements

To make it possible for replacements made to headers as part of transforming
one translation unit to not be visible to the transform of other translation
units, Transform now handles replacement application as part of its
end-of-source handling. Several things were simplified as a result:

- The duplicated code in every transform for applying replacements is now gone
  and replaced with one location in Transform.
- RefactoringTool is no longer used since Transform houses the Replacements
  structure.
- RewriterContainer is now a private implementation detail of Transform (also
  renamed to RewriterManager since its behaviour is slightly different now with
  respect to lifetime of objects).
- There's now no distinction between input and output file state.

Misc notes:

- Interface changes reflected in unit tests.
- Replacements for files other than the main file are assumed to be for headers
  and stored as such.

llvm-svn: 184194

5 files changed, 141 insertions, 98 deletions

diff --git a/clang-tools-extra/cpp11-migrate/Core/FileOverrides.cpp b/clang-tools-extra/cpp11-migrate/Core/FileOverrides.cpp
index 1c3320780bb..81fca416157 100644
--- a/clang-tools-extra/cpp11-migrate/Core/FileOverrides.cpp
+++ b/clang-tools-extra/cpp11-migrate/Core/FileOverrides.cpp
@@ -1,16 +1,18 @@
 #include "Core/FileOverrides.h"
 #include "clang/Basic/SourceManager.h"
 
-void SourceOverrides::applyOverrides(clang::SourceManager &SM,
-                                     clang::FileManager &FM) const {
-  assert(!MainFileOverride.empty() &&
-         "Main source file override should exist!");
-  SM.overrideFileContents(FM.getFile(MainFileName),
-                          llvm::MemoryBuffer::getMemBuffer(MainFileOverride));
+void SourceOverrides::applyOverrides(clang::SourceManager &SM) const {
+  clang::FileManager &FM = SM.getFileManager();
+
+  if (isSourceOverriden())
+    SM.overrideFileContents(FM.getFile(MainFileName),
+                            llvm::MemoryBuffer::getMemBuffer(MainFileOverride));
 
   for (HeaderOverrides::const_iterator I = Headers.begin(),
-       E = Headers.end(); I != E; ++I)
+       E = Headers.end(); I != E; ++I) {
+    assert(!I->second.FileOverride.empty() && "Header override should not be empty!");
     SM.overrideFileContents(
         FM.getFile(I->second.FileName),
         llvm::MemoryBuffer::getMemBuffer(I->second.FileOverride));
+  }
 }
diff --git a/clang-tools-extra/cpp11-migrate/Core/FileOverrides.h b/clang-tools-extra/cpp11-migrate/Core/FileOverrides.h
index 03d70ebb13a..5f6988c646c 100644
--- a/clang-tools-extra/cpp11-migrate/Core/FileOverrides.h
+++ b/clang-tools-extra/cpp11-migrate/Core/FileOverrides.h
@@ -26,6 +26,8 @@ class FileManager;
 
 /// \brief Container for storing override information for a single headers.
 struct HeaderOverride {
+  HeaderOverride(const char *FileName) : FileName(FileName) {}
+
   std::string FileName;
   std::string FileOverride;
 };
@@ -35,12 +37,18 @@ typedef std::map<std::string, HeaderOverride> HeaderOverrides;
 
 /// \brief Container storing the file content overrides for a source file.
 struct SourceOverrides {
-  SourceOverrides(const char *MainFileName)
+  SourceOverrides(const std::string &MainFileName)
       : MainFileName(MainFileName) {}
 
   /// \brief Convenience function for applying this source's overrides to
   /// the given SourceManager.
-  void applyOverrides(clang::SourceManager &SM, clang::FileManager &FM) const;
+  void applyOverrides(clang::SourceManager &SM) const;
+
+  /// \brief Indicates if the source file has been overridden.
+  ///
+  /// It's possible for a source to remain unchanged while only headers are
+  /// changed.
+  bool isSourceOverriden() const { return !MainFileOverride.empty(); }
 
   std::string MainFileName;
   std::string MainFileOverride;
diff --git a/clang-tools-extra/cpp11-migrate/Core/SyntaxCheck.cpp b/clang-tools-extra/cpp11-migrate/Core/SyntaxCheck.cpp
index 65877668ee4..8c29c816a88 100644
--- a/clang-tools-extra/cpp11-migrate/Core/SyntaxCheck.cpp
+++ b/clang-tools-extra/cpp11-migrate/Core/SyntaxCheck.cpp
@@ -16,7 +16,7 @@ public:
 
     FileOverrides::const_iterator I = Overrides.find(Filename);
     if (I != Overrides.end())
-      I->second.applyOverrides(CI.getSourceManager(), CI.getFileManager());
+      I->second.applyOverrides(CI.getSourceManager());
 
     return true;
   }
diff --git a/clang-tools-extra/cpp11-migrate/Core/Transform.cpp b/clang-tools-extra/cpp11-migrate/Core/Transform.cpp
index adc6e6dc718..5fd73ad0380 100644
--- a/clang-tools-extra/cpp11-migrate/Core/Transform.cpp
+++ b/clang-tools-extra/cpp11-migrate/Core/Transform.cpp
@@ -1,6 +1,10 @@
 #include "Core/Transform.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/raw_ostream.h"
@@ -54,29 +58,55 @@ private:
   MatchFinder &Finder;
   Transform &Owner;
 };
-
 } // namespace
 
-RewriterContainer::RewriterContainer(clang::FileManager &Files,
-                                     const FileOverrides &InputStates)
-    : 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) {
-  for (FileOverrides::const_iterator I = InputStates.begin(),
-                                       E = InputStates.end();
-       I != E; ++I)
-    I->second.applyOverrides(Sources, Files);
-}
+/// \brief Class for creating Rewriter objects and housing Rewriter
+/// dependencies.
+///
+/// A Rewriter depends on a SourceManager which in turn depends on a
+/// FileManager and a DiagnosticsEngine. Transform uses this class to create a
+/// new Rewriter and SourceManager for every translation unit it transforms. A
+/// DiagnosticsEngine doesn't need to be re-created so it's constructed once. A
+/// SourceManager and Rewriter and (re)created as required.
+///
+/// FIXME: The DiagnosticsEngine should really come from somewhere more global.
+/// It shouldn't be re-created once for every transform.
+///
+/// NOTE: SourceManagers cannot be shared. Therefore the one used to parse the
+/// translation unit cannot be used to create a Rewriter. This is why both a
+/// SourceManager and Rewriter need to be created for each translation unit.
+class RewriterManager {
+public:
+  RewriterManager()
+      : DiagOpts(new DiagnosticOptions()),
+        DiagnosticPrinter(llvm::errs(), DiagOpts.getPtr()),
+        Diagnostics(
+            llvm::IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs()),
+            DiagOpts.getPtr(), &DiagnosticPrinter, false) {}
+
+  void prepare(FileManager &Files) {
+    Sources.reset(new SourceManager(Diagnostics, Files));
+    Rewrite.reset(new Rewriter(*Sources, DefaultLangOptions));
+  }
 
-void collectResults(clang::Rewriter &Rewrite,
-                    const FileOverrides &InputStates,
-                    FileOverrides &Results) {
-  // Copy the contents of InputStates to be modified.
-  Results = InputStates;
+  void applyOverrides(const SourceOverrides &Overrides) {
+    Overrides.applyOverrides(*Sources);
+  }
+
+  Rewriter &getRewriter() { return *Rewrite; }
 
+private:
+  LangOptions DefaultLangOptions;
+  llvm::IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts;
+  TextDiagnosticPrinter DiagnosticPrinter;
+  DiagnosticsEngine Diagnostics;
+  llvm::OwningPtr<SourceManager> Sources;
+  llvm::OwningPtr<Rewriter> Rewrite;
+};
+
+/// \brief Flatten the Rewriter buffers of \p Rewrite and store results as
+/// file content overrides in \p Overrides.
+void collectResults(clang::Rewriter &Rewrite, SourceOverrides &Overrides) {
   for (Rewriter::buffer_iterator I = Rewrite.buffer_begin(),
                                  E = Rewrite.buffer_end();
        I != E; ++I) {
@@ -85,12 +115,6 @@ void collectResults(clang::Rewriter &Rewrite,
     assert(Entry->getName() != 0 &&
            "Unexpected NULL return from FileEntry::getName()");
 
-    FileOverrides::iterator OverrideI = Results.find(Entry->getName());
-    if (OverrideI == Results.end()) {
-      OverrideI = Results.insert(FileOverrides::value_type(
-          Entry->getName(), Entry->getName())).first;
-    }
-
     std::string ResultBuf;
 
     // Get a copy of the rewritten buffer from the Rewriter.
@@ -103,18 +127,49 @@ void collectResults(clang::Rewriter &Rewrite,
     // FIXME: Use move semantics to avoid copies of the buffer contents if
     // benchmarking shows the copies are expensive, especially for large source
     // files.
-    OverrideI->second.MainFileOverride = ResultBuf;
+
+    if (Overrides.MainFileName == Entry->getName()) {
+      Overrides.MainFileOverride = ResultBuf;
+      continue;
+    }
+
+    // Header overrides are treated differently. Eventually, raw replacements
+    // will be stored as well for later output to disk. Applying replacements
+    // in memory will always be necessary as the source goes down the transform
+    // pipeline.
+
+    HeaderOverrides &Headers = Overrides.Headers;
+    HeaderOverrides::iterator HeaderI = Headers.find(Entry->getName());
+    if (HeaderI == Headers.end())
+      HeaderI = Headers.insert(HeaderOverrides::value_type(
+          Entry->getName(), Entry->getName())).first;
+
+    HeaderI->second.FileOverride = ResultBuf;
   }
 }
 
+Transform::Transform(llvm::StringRef Name, const TransformOptions &Options)
+    : Name(Name), GlobalOptions(Options), Overrides(0),
+      RewriterOwner(new RewriterManager) {
+  Reset();
+}
+
+Transform::~Transform() {}
+
 bool Transform::handleBeginSource(CompilerInstance &CI, StringRef Filename) {
-  assert(InputState != 0 && "Subclass transform didn't provide InputState");
+  assert(Overrides != 0 && "Subclass transform didn't provide InputState");
+
+  CurrentSource = Filename.str();
 
-  FileOverrides::const_iterator I = InputState->find(Filename.str());
-  if (I != InputState->end()) {
-    I->second.applyOverrides(CI.getSourceManager(), CI.getFileManager());
+  RewriterOwner->prepare(CI.getFileManager());
+  FileOverrides::const_iterator I = Overrides->find(CurrentSource);
+  if (I != Overrides->end()) {
+    I->second.applyOverrides(CI.getSourceManager());
+    RewriterOwner->applyOverrides(I->second);
   }
 
+  Replace.clear();
+
   if (Options().EnableTiming) {
     Timings.push_back(std::make_pair(Filename.str(), llvm::TimeRecord()));
     Timings.back().second -= llvm::TimeRecord::getCurrentTime(true);
@@ -123,10 +178,21 @@ bool Transform::handleBeginSource(CompilerInstance &CI, StringRef Filename) {
 }
 
 void Transform::handleEndSource() {
-  if (!Options().EnableTiming)
-    return;
+  if (!getReplacements().empty()) {
+    // FIXME: applyAllReplacements will indicate if it couldn't apply all
+    // replacements. Handle that case.
+    applyAllReplacements(getReplacements(), RewriterOwner->getRewriter());
+
+    FileOverrides::iterator I = Overrides->find(CurrentSource);
+    if (I == Overrides->end())
+      I = Overrides
+        ->insert(FileOverrides::value_type(CurrentSource, CurrentSource)).first;
+
+    collectResults(RewriterOwner->getRewriter(), I->second);
+  }
 
-  Timings.back().second += llvm::TimeRecord::getCurrentTime(false);
+  if (Options().EnableTiming)
+    Timings.back().second += llvm::TimeRecord::getCurrentTime(false);
 }
 
 void Transform::addTiming(llvm::StringRef Label, llvm::TimeRecord Duration) {
diff --git a/clang-tools-extra/cpp11-migrate/Core/Transform.h b/clang-tools-extra/cpp11-migrate/Core/Transform.h
index d9135e78e2f..651ee32e65f 100644
--- a/clang-tools-extra/cpp11-migrate/Core/Transform.h
+++ b/clang-tools-extra/cpp11-migrate/Core/Transform.h
@@ -19,16 +19,9 @@
 #include <vector>
 #include "Core/IncludeExcludeInfo.h"
 #include "Core/FileOverrides.h"
+#include "clang/Tooling/Refactoring.h"
 #include "llvm/Support/Timer.h"
-
-// 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"
-////
+#include "llvm/ADT/OwningPtr.h"
 
 
 /// \brief Description of the riskiness of actions that can be taken by
@@ -56,41 +49,7 @@ class MatchFinder;
 } // namespace ast_matchers
 } // namespace clang
 
-
-/// \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,
-                    const FileOverrides &InputStates,
-                    FileOverrides &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,
-                    const FileOverrides &InputStates);
-
-  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;
-};
+class RewriterManager;
 
 /// \brief Container for global options affecting all transforms.
 struct TransformOptions {
@@ -125,12 +84,10 @@ public:
   /// \param Name Name of the transform for human-readable purposes (e.g. -help
   /// text)
   /// \param Options Collection of options that affect all transforms.
-  Transform(llvm::StringRef Name, const TransformOptions &Options)
-      : Name(Name), GlobalOptions(Options), InputState(0) {
-    Reset();
-  }
+  /// \param InitialState File Contents to override content on disk.
+  Transform(llvm::StringRef Name, const TransformOptions &Options);
 
-  virtual ~Transform() {}
+  virtual ~Transform();
 
   /// \brief Apply a transform to all files listed in \p SourcePaths.
   ///
@@ -139,10 +96,9 @@ public:
   /// 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 FileOverrides &InputStates,
+  virtual int apply(FileOverrides &InputStates,
                     const clang::tooling::CompilationDatabase &Database,
-                    const std::vector<std::string> &SourcePaths,
-                    FileOverrides &ResultStates) = 0;
+                    const std::vector<std::string> &SourcePaths) = 0;
 
   /// \brief Query if changes were made during the last call to apply().
   bool getChangesMade() const { return AcceptedChanges > 0; }
@@ -221,14 +177,22 @@ protected:
   /// data for all sources processed by this transform.
   void addTiming(llvm::StringRef Label, llvm::TimeRecord Duration);
 
+  /// \brief Provide access for subclasses to the TransformOptions they were
+  /// created with.
   const TransformOptions &Options() { return GlobalOptions; }
 
-  /// \brief Allows a subclass to provide file contents overrides before
+  /// \brief Provide access for subclasses for the container to store
+  /// translation unit replacements.
+  clang::tooling::Replacements &getReplacements() { return Replace; }
+
+  /// \brief Affords a subclass to provide file contents overrides before
   /// applying frontend actions.
   ///
   /// It is an error not to call this function before calling ClangTool::run()
   /// with the factory provided by createActionFactory().
-  void setOverrides(const FileOverrides &Overrides) { InputState = &Overrides; }
+  void setOverrides(FileOverrides &Overrides) {
+    this->Overrides = &Overrides;
+  }
 
   /// \brief Subclasses must call this function to create a
   /// FrontendActionFactory to pass to ClangTool.
@@ -241,8 +205,11 @@ protected:
 private:
   const std::string Name;
   const TransformOptions &GlobalOptions;
+  FileOverrides *Overrides;
+  clang::tooling::Replacements Replace;
+  llvm::OwningPtr<RewriterManager> RewriterOwner;
+  std::string CurrentSource;
   TimingVec Timings;
-  const FileOverrides *InputState;
   unsigned AcceptedChanges;
   unsigned RejectedChanges;
   unsigned DeferredChanges;