[clang-tools-extra] r184098 - cpp11-migrate: Transform now responsible for file content overriding

Edwin Vane edwin.vane at intel.com
Mon Jun 17 11:18:16 PDT 2013


Author: revane
Date: Mon Jun 17 13:18:15 2013
New Revision: 184098

URL: http://llvm.org/viewvc/llvm-project?rev=184098&view=rev
Log:
cpp11-migrate: Transform now responsible for file content overriding

To better support per-translation unit replacements, any real work is being
moved out of ActionFactory and into Transform. In this revision, that means
file override application.

For simplification, Transform no longer inherits from SourceFileCallbacks.
TransformTest required updating as a result.


Modified:
    clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverride.cpp
    clang-tools-extra/trunk/cpp11-migrate/Core/Transform.cpp
    clang-tools-extra/trunk/cpp11-migrate/Core/Transform.h
    clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopConvert.cpp
    clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAuto.cpp
    clang-tools-extra/trunk/cpp11-migrate/UseNullptr/UseNullptr.cpp
    clang-tools-extra/trunk/unittests/cpp11-migrate/TransformTest.cpp

Modified: clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverride.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverride.cpp?rev=184098&r1=184097&r2=184098&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverride.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverride.cpp Mon Jun 17 13:18:15 2013
@@ -43,13 +43,14 @@ int AddOverrideTransform::apply(const Fi
   MatchFinder Finder;
   AddOverrideFixer Fixer(AddOverrideTool.getReplacements(), AcceptedChanges,
                          DetectMacros);
+  Finder.addMatcher(makeCandidateForOverrideAttrMatcher(), &Fixer);
+
   // Make Fixer available to handleBeginSource().
   this->Fixer = &Fixer;
 
-  Finder.addMatcher(makeCandidateForOverrideAttrMatcher(), &Fixer);
+  setOverrides(InputStates);
 
-  if (int result =
-          AddOverrideTool.run(createActionFactory(Finder, InputStates))) {
+  if (int result = AddOverrideTool.run(createActionFactory(Finder))) {
     llvm::errs() << "Error encountered during translation.\n";
     return result;
   }

Modified: clang-tools-extra/trunk/cpp11-migrate/Core/Transform.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/Core/Transform.cpp?rev=184098&r1=184097&r2=184098&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/Core/Transform.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/Core/Transform.cpp Mon Jun 17 13:18:15 2013
@@ -2,6 +2,7 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
@@ -11,29 +12,22 @@ namespace {
 using namespace tooling;
 using namespace ast_matchers;
 
-/// \brief Custom FrontendActionFactory to produce FrontendActions that handle
-/// overriding source file contents before parsing.
-///
-/// The nested class FactoryAdaptor overrides BeginSourceFileAction to override
-/// source file contents before parsing happens. Both Begin and
-/// EndSourceFileAction call corresponding callbacks provided by
-/// SourceFileCallbacks.
+/// \brief Custom FrontendActionFactory to produce FrontendActions that simply
+/// forward (Begin|End)SourceFileAction calls to a given Transform.
 class ActionFactory : public clang::tooling::FrontendActionFactory {
 public:
-  ActionFactory(MatchFinder &Finder, const FileOverrides &Overrides,
-                SourceFileCallbacks &Callbacks)
-  : Finder(Finder), Overrides(Overrides), Callbacks(Callbacks) {}
+  ActionFactory(MatchFinder &Finder, Transform &Owner)
+  : Finder(Finder), Owner(Owner) {}
 
   virtual FrontendAction *create() LLVM_OVERRIDE {
-    return new FactoryAdaptor(Finder, Overrides, Callbacks);
+    return new FactoryAdaptor(Finder, Owner);
   }
 
 private:
   class FactoryAdaptor : public ASTFrontendAction {
   public:
-    FactoryAdaptor(MatchFinder &Finder, const FileOverrides &Overrides,
-                  SourceFileCallbacks &Callbacks)
-        : Finder(Finder), Overrides(Overrides), Callbacks(Callbacks) {}
+    FactoryAdaptor(MatchFinder &Finder, Transform &Owner)
+        : Finder(Finder), Owner(Owner) {}
 
     ASTConsumer *CreateASTConsumer(CompilerInstance &, StringRef) {
       return Finder.newASTConsumer();
@@ -44,28 +38,21 @@ private:
       if (!ASTFrontendAction::BeginSourceFileAction(CI, Filename))
         return false;
 
-      FileOverrides::const_iterator I = Overrides.find(Filename.str());
-      if (I != Overrides.end()) {
-        I->second.applyOverrides(CI.getSourceManager(), CI.getFileManager());
-      }
-
-      return Callbacks.handleBeginSource(CI, Filename);
+      return Owner.handleBeginSource(CI, Filename);
     }
 
     virtual void EndSourceFileAction() LLVM_OVERRIDE {
-      Callbacks.handleEndSource();
+      Owner.handleEndSource();
       return ASTFrontendAction::EndSourceFileAction();
     }
 
   private:
     MatchFinder &Finder;
-    const FileOverrides &Overrides;
-    SourceFileCallbacks &Callbacks;
+    Transform &Owner;
   };
 
   MatchFinder &Finder;
-  const FileOverrides &Overrides;
-  SourceFileCallbacks &Callbacks;
+  Transform &Owner;
 };
 
 } // namespace
@@ -121,11 +108,17 @@ void collectResults(clang::Rewriter &Rew
 }
 
 bool Transform::handleBeginSource(CompilerInstance &CI, StringRef Filename) {
-  if (!Options().EnableTiming)
-    return true;
+  assert(InputState != 0 && "Subclass transform didn't provide InputState");
 
-  Timings.push_back(std::make_pair(Filename.str(), llvm::TimeRecord()));
-  Timings.back().second -= llvm::TimeRecord::getCurrentTime(true);
+  FileOverrides::const_iterator I = InputState->find(Filename.str());
+  if (I != InputState->end()) {
+    I->second.applyOverrides(CI.getSourceManager(), CI.getFileManager());
+  }
+
+  if (Options().EnableTiming) {
+    Timings.push_back(std::make_pair(Filename.str(), llvm::TimeRecord()));
+    Timings.back().second -= llvm::TimeRecord::getCurrentTime(true);
+  }
   return true;
 }
 
@@ -141,7 +134,6 @@ void Transform::addTiming(llvm::StringRe
 }
 
 FrontendActionFactory *
-Transform::createActionFactory(MatchFinder &Finder,
-                               const FileOverrides &InputStates) {
-  return new ActionFactory(Finder, InputStates, *this);
+Transform::createActionFactory(MatchFinder &Finder) {
+  return new ActionFactory(Finder, /*Owner=*/ *this);
 }

Modified: clang-tools-extra/trunk/cpp11-migrate/Core/Transform.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/Core/Transform.h?rev=184098&r1=184097&r2=184098&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/Core/Transform.h (original)
+++ clang-tools-extra/trunk/cpp11-migrate/Core/Transform.h Mon Jun 17 13:18:15 2013
@@ -19,8 +19,6 @@
 #include <vector>
 #include "Core/IncludeExcludeInfo.h"
 #include "Core/FileOverrides.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Timer.h"
 
 // For RewriterContainer
@@ -48,8 +46,10 @@ enum RiskLevel {
 
 // Forward declarations
 namespace clang {
+class CompilerInstance;
 namespace tooling {
 class CompilationDatabase;
+class FrontendActionFactory;
 } // namespace tooling
 namespace ast_matchers {
 class MatchFinder;
@@ -112,21 +112,21 @@ struct TransformOptions {
 
 /// \brief Abstract base class for all C++11 migration transforms.
 ///
-/// Per-source performance timing is handled by the callbacks
-/// handleBeginSource() and handleEndSource() if timing is enabled. See
-/// clang::tooling::newFrontendActionFactory() for how to register a Transform
-/// object for callbacks. When a Transform object is registered for
-/// FrontendAction source file callbacks, this behaviour can be used to time
-/// the application of a MatchFinder by subclasses. Durations are automatically
-/// stored in a TimingVec.
-class Transform : public clang::tooling::SourceFileCallbacks {
+/// Subclasses must call createActionFactory() to create a
+/// FrontendActionFactory to pass to ClangTool::run(). Subclasses are also
+/// responsible for calling setOverrides() before calling ClangTool::run().
+///
+/// If timing is enabled (see TransformOptions), per-source performance timing
+/// is recorded and stored in a TimingVec for later access with timing_begin()
+/// and timing_end().
+class Transform {
 public:
   /// \brief Constructor
   /// \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) {
+      : Name(Name), GlobalOptions(Options), InputState(0) {
     Reset();
   }
 
@@ -175,17 +175,21 @@ public:
     DeferredChanges = 0;
   }
 
-  /// \brief Callback for notification of the start of processing of a source
-  /// file by a FrontendAction. Starts a performance timer if timing was
-  /// enabled.
+  /// \brief Called before parsing a translation unit for a FrontendAction.
+  ///
+  /// Transform uses this function to apply file overrides and start
+  /// performance timers. Subclasses overriding this function must call it
+  /// before returning.
   virtual bool handleBeginSource(clang::CompilerInstance &CI,
-                                 llvm::StringRef Filename) LLVM_OVERRIDE;
+                                 llvm::StringRef Filename);
 
-  /// \brief Callback for notification of the end of processing of a source
-  /// file by a FrontendAction. Stops a performance timer if timing was enabled
-  /// and records the elapsed time. For a given source, handleBeginSource() and
-  /// handleEndSource() are expected to be called in pairs.
-  virtual void handleEndSource() LLVM_OVERRIDE;
+  /// \brief Called after FrontendAction has been run over a translation unit.
+  ///
+  /// Transform uses this function to stop performance timers. Subclasses
+  /// overriding this function must call it before returning. A call to
+  /// handleEndSource() for a given translation unit is expected to be called
+  /// immediately after the corresponding handleBeginSource() call.
+  virtual void handleEndSource();
 
   /// \brief Performance timing data is stored as a vector of pairs. Pairs are
   /// formed of:
@@ -219,17 +223,26 @@ protected:
 
   const TransformOptions &Options() { return GlobalOptions; }
 
-  /// \brief Subclasses call this function to create a FrontendActionFactory to
-  /// pass to ClangTool. The factory returned by this function is responsible
-  /// for overriding source file contents with results of previous transforms.
+  /// \brief Allows 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; }
+
+  /// \brief Subclasses must call this function to create a
+  /// FrontendActionFactory to pass to ClangTool.
+  ///
+  /// The factory returned by this function is responsible for calling back to
+  /// Transform to call handleBeginSource() and handleEndSource().
   clang::tooling::FrontendActionFactory *
-      createActionFactory(clang::ast_matchers::MatchFinder &Finder,
-                          const FileOverrides &InputStates);
+      createActionFactory(clang::ast_matchers::MatchFinder &Finder);
 
 private:
   const std::string Name;
   const TransformOptions &GlobalOptions;
   TimingVec Timings;
+  const FileOverrides *InputState;
   unsigned AcceptedChanges;
   unsigned RejectedChanges;
   unsigned DeferredChanges;

Modified: clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopConvert.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopConvert.cpp?rev=184098&r1=184097&r2=184098&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopConvert.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/LoopConvert/LoopConvert.cpp Mon Jun 17 13:18:15 2013
@@ -57,7 +57,9 @@ int LoopConvertTransform::apply(const Fi
                                   Options().MaxRiskLevel, LFK_PseudoArray);
   Finder.addMatcher(makePseudoArrayLoopMatcher(), &PseudoarrrayLoopFixer);
 
-  if (int result = LoopTool.run(createActionFactory(Finder, InputStates))) {
+  setOverrides(InputStates);
+
+  if (int result = LoopTool.run(createActionFactory(Finder))) {
     llvm::errs() << "Error encountered during translation.\n";
     return result;
   }

Modified: clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAuto.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAuto.cpp?rev=184098&r1=184097&r2=184098&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAuto.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAuto.cpp Mon Jun 17 13:18:15 2013
@@ -37,7 +37,9 @@ int UseAutoTransform::apply(const FileOv
   Finder.addMatcher(makeIteratorDeclMatcher(), &ReplaceIterators);
   Finder.addMatcher(makeDeclWithNewMatcher(), &ReplaceNew);
 
-  if (int Result = UseAutoTool.run(createActionFactory(Finder, InputStates))) {
+  setOverrides(InputStates);
+
+  if (int Result = UseAutoTool.run(createActionFactory(Finder))) {
     llvm::errs() << "Error encountered during translation.\n";
     return Result;
   }

Modified: clang-tools-extra/trunk/cpp11-migrate/UseNullptr/UseNullptr.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/UseNullptr/UseNullptr.cpp?rev=184098&r1=184097&r2=184098&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/UseNullptr/UseNullptr.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/UseNullptr/UseNullptr.cpp Mon Jun 17 13:18:15 2013
@@ -37,11 +37,11 @@ int UseNullptrTransform::apply(const Fil
   NullptrFixer Fixer(UseNullptrTool.getReplacements(),
                      AcceptedChanges,
                      Options().MaxRiskLevel);
-
   Finder.addMatcher(makeCastSequenceMatcher(), &Fixer);
 
-  if (int result =
-          UseNullptrTool.run(createActionFactory(Finder, InputStates))) {
+  setOverrides(InputStates);
+
+  if (int result = UseNullptrTool.run(createActionFactory(Finder))) {
     llvm::errs() << "Error encountered during translation.\n";
     return result;
   }

Modified: clang-tools-extra/trunk/unittests/cpp11-migrate/TransformTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/cpp11-migrate/TransformTest.cpp?rev=184098&r1=184097&r2=184098&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/cpp11-migrate/TransformTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/cpp11-migrate/TransformTest.cpp Mon Jun 17 13:18:15 2013
@@ -2,6 +2,7 @@
 #include "Core/Transform.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PathV1.h"
@@ -27,6 +28,11 @@ public:
   void setDeferredChanges(unsigned Changes) {
     Transform::setDeferredChanges(Changes);
   }
+
+  void setOverrides(const FileOverrides &Overrides) {
+    Transform::setOverrides(Overrides);
+  }
+
 };
 
 TEST(Transform, Interface) {
@@ -58,9 +64,9 @@ TEST(Transform, Interface) {
   ASSERT_TRUE(T.getChangesNotMade());
 }
 
-class FindTopLevelDeclConsumer : public ASTConsumer {
+class TimePassingASTConsumer : public ASTConsumer {
 public:
-  FindTopLevelDeclConsumer(bool *Called) : Called(Called) {}
+  TimePassingASTConsumer(bool *Called) : Called(Called) {}
 
   virtual bool HandleTopLevelDecl(DeclGroupRef DeclGroup) {
     llvm::sys::TimeValue UserStart;
@@ -83,11 +89,25 @@ public:
 
 struct ConsumerFactory {
   ASTConsumer *newASTConsumer() {
-    return new FindTopLevelDeclConsumer(&Called);
+    return new TimePassingASTConsumer(&Called);
   }
   bool Called;
 };
 
+struct CallbackForwarder : public clang::tooling::SourceFileCallbacks {
+  CallbackForwarder(Transform &Callee) : Callee(Callee) {}
+
+  virtual bool handleBeginSource(CompilerInstance &CI, StringRef Filename) {
+    return Callee.handleBeginSource(CI, Filename);
+  }
+
+  virtual void handleEndSource() {
+    Callee.handleEndSource();
+  }
+
+  Transform &Callee;
+};
+
 TEST(Transform, Timings) {
   TransformOptions Options;
   Options.EnableTiming = true;
@@ -115,8 +135,21 @@ TEST(Transform, Timings) {
   Tool.mapVirtualFile(FileAName, "void a() {}");
   Tool.mapVirtualFile(FileBName, "void b() {}");
 
+  // Factory to create TimePassingASTConsumer for each source file the tool
+  // runs on.
   ConsumerFactory Factory;
-  Tool.run(newFrontendActionFactory(&Factory, &T));
+
+  // We don't care about any of Transform's functionality except to get it to
+  // record timings. For that, we need to forward handleBeginSource() and
+  // handleEndSource() calls to it.
+  CallbackForwarder Callbacks(T);
+
+  // Transform's handle* functions require FileOverrides to be set, even if
+  // there aren't any.
+  FileOverrides Overrides;
+  T.setOverrides(Overrides);
+
+  Tool.run(clang::tooling::newFrontendActionFactory(&Factory, &Callbacks));
 
   EXPECT_TRUE(Factory.Called);
   Transform::TimingVec::const_iterator I = T.timing_begin();





More information about the cfe-commits mailing list