cpp11-migrate: Refactor how global options are passed to Transforms

Refactored how global options are passed to Transforms to avoid widespread
changes every time a new global option is added.

Tests updated to reflect new interface.

llvm-svn: 183443

15 files changed, 73 insertions, 61 deletions

diff --git a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.cpp b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.cpp
index f02548b25ff..df0ed10d9bd 100644
--- a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.cpp
+++ b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.cpp
@@ -32,7 +32,6 @@ static llvm::cl::opt<bool> DetectMacros(
     llvm::cl::desc("Detect and use macros that expand to the 'override' keyword."));
 
 int AddOverrideTransform::apply(const FileContentsByPath &InputStates,
-                                RiskLevel MaxRisk,
                                 const CompilationDatabase &Database,
                                 const std::vector<std::string> &SourcePaths,
                                 FileContentsByPath &ResultStates) {
diff --git a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.h b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.h
index 45bd6ee8725..976807cf6f2 100644
--- a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.h
+++ b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.h
@@ -26,12 +26,11 @@ class AddOverrideFixer;
 /// member functions overriding base class virtual functions.
 class AddOverrideTransform : public Transform {
 public:
-  AddOverrideTransform(bool EnableTiming)
-      : Transform("AddOverride", EnableTiming) {}
+  AddOverrideTransform(const TransformOptions &Options)
+      : Transform("AddOverride", Options) {}
 
   /// \see Transform::run().
   virtual int apply(const FileContentsByPath &InputStates,
-                    RiskLevel MaxRiskLEvel,
                     const clang::tooling::CompilationDatabase &Database,
                     const std::vector<std::string> &SourcePaths,
                     FileContentsByPath &ResultStates) LLVM_OVERRIDE;
diff --git a/clang-tools-extra/cpp11-migrate/Core/Transform.cpp b/clang-tools-extra/cpp11-migrate/Core/Transform.cpp
index cfb316bb0b0..97597d63047 100644
--- a/clang-tools-extra/cpp11-migrate/Core/Transform.cpp
+++ b/clang-tools-extra/cpp11-migrate/Core/Transform.cpp
@@ -37,7 +37,7 @@ void collectResults(clang::Rewriter &Rewrite,
 }
 
 bool Transform::handleBeginSource(CompilerInstance &CI, StringRef Filename) {
-  if (!EnableTiming)
+  if (!Options().EnableTiming)
     return true;
 
   Timings.push_back(std::make_pair(Filename.str(), llvm::TimeRecord()));
@@ -46,7 +46,7 @@ bool Transform::handleBeginSource(CompilerInstance &CI, StringRef Filename) {
 }
 
 void Transform::handleEndSource() {
-  if (!EnableTiming)
+  if (!Options().EnableTiming)
     return;
 
   Timings.back().second += llvm::TimeRecord::getCurrentTime(false);
diff --git a/clang-tools-extra/cpp11-migrate/Core/Transform.h b/clang-tools-extra/cpp11-migrate/Core/Transform.h
index 1297ec1becd..264b59526a7 100644
--- a/clang-tools-extra/cpp11-migrate/Core/Transform.h
+++ b/clang-tools-extra/cpp11-migrate/Core/Transform.h
@@ -17,6 +17,7 @@
 
 #include <string>
 #include <vector>
+#include "IncludeExcludeInfo.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Timer.h"
 
@@ -105,24 +106,32 @@ private:
   clang::Rewriter Rewrite;
 };
 
+/// \brief Container for global options affecting all transforms.
+struct TransformOptions {
+  /// \brief Enable the use of performance timers.
+  bool EnableTiming;
+
+  /// \brief Maximum allowed level of risk.
+  RiskLevel MaxRiskLevel;
+};
+
 /// \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.
+/// 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 {
 public:
   /// \brief Constructor
   /// \param Name Name of the transform for human-readable purposes (e.g. -help
   /// text)
-  /// \param EnableTiming Enable the timing of the duration between calls to
-  /// handleBeginSource() and handleEndSource(). 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.
-  Transform(llvm::StringRef Name, bool EnableTiming)
-      : Name(Name), EnableTiming(EnableTiming) {
+  /// \param Options Collection of options that affect all transforms.
+  Transform(llvm::StringRef Name, const TransformOptions &Options)
+      : Name(Name), GlobalOptions(Options) {
     Reset();
   }
 
@@ -136,7 +145,6 @@ public:
   /// 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,
                     FileContentsByPath &ResultStates) = 0;
@@ -214,9 +222,11 @@ protected:
   /// data for all sources processed by this transform.
   void addTiming(llvm::StringRef Label, llvm::TimeRecord Duration);
 
+  const TransformOptions &Options() { return GlobalOptions; }
+
 private:
   const std::string Name;
-  bool EnableTiming;
+  const TransformOptions &GlobalOptions;
   TimingVec Timings;
   unsigned AcceptedChanges;
   unsigned RejectedChanges;
diff --git a/clang-tools-extra/cpp11-migrate/Core/Transforms.cpp b/clang-tools-extra/cpp11-migrate/Core/Transforms.cpp
index 85c0e2103a7..4347e4e46df 100644
--- a/clang-tools-extra/cpp11-migrate/Core/Transforms.cpp
+++ b/clang-tools-extra/cpp11-migrate/Core/Transforms.cpp
@@ -32,14 +32,16 @@ void Transforms::registerTransform(llvm::StringRef OptName,
                                    llvm::StringRef Description,
                                    TransformCreator Creator) {
   Options.push_back(OptionVec::value_type(
-      new cl::opt<bool>(OptName.data(), cl::desc(Description.data())), Creator));
+      new cl::opt<bool>(OptName.data(), cl::desc(Description.data())),
+      Creator));
 }
 
-void Transforms::createSelectedTransforms(bool EnableTiming) {
+void
+Transforms::createSelectedTransforms(const TransformOptions &GlobalOptions) {
   for (OptionVec::iterator I = Options.begin(),
        E = Options.end(); I != E; ++I) {
     if (*I->first) {
-      ChosenTransforms.push_back(I->second(EnableTiming));
+      ChosenTransforms.push_back(I->second(GlobalOptions));
     }
   }
 }
diff --git a/clang-tools-extra/cpp11-migrate/Core/Transforms.h b/clang-tools-extra/cpp11-migrate/Core/Transforms.h
index b19b0316d90..c07df992574 100644
--- a/clang-tools-extra/cpp11-migrate/Core/Transforms.h
+++ b/clang-tools-extra/cpp11-migrate/Core/Transforms.h
@@ -27,11 +27,12 @@ class Option;
 } // namespace cl
 } // namespace llvm
 class Transform;
+struct TransformOptions;
 
-typedef Transform *(*TransformCreator)(bool);
+typedef Transform *(*TransformCreator)(const TransformOptions &);
 template <typename T>
-Transform *ConstructTransform(bool EnableTiming) {
-  return new T(EnableTiming);
+Transform *ConstructTransform(const TransformOptions &Options) {
+  return new T(Options);
 }
 
 /// \brief Class encapsulating the creation of command line bool options
@@ -55,7 +56,7 @@ public:
   /// \brief Instantiate all transforms that were selected on the command line.
   ///
   /// Call *after* parsing options.
-  void createSelectedTransforms(bool EnableTiming);
+  void createSelectedTransforms(const TransformOptions &Options);
 
   /// \brief Return an iterator to the start of a container of instantiated
   /// transforms.
diff --git a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.cpp b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.cpp
index 9afd4990e45..38659f854fa 100644
--- a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.cpp
+++ b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.cpp
@@ -26,7 +26,6 @@ using namespace clang::tooling;
 using namespace clang;
 
 int LoopConvertTransform::apply(const FileContentsByPath &InputStates,
-                                RiskLevel MaxRisk,
                                 const CompilationDatabase &Database,
                                 const std::vector<std::string> &SourcePaths,
                                 FileContentsByPath &ResultStates) {
@@ -49,19 +48,19 @@ int LoopConvertTransform::apply(const FileContentsByPath &InputStates,
   LoopFixer ArrayLoopFixer(&ParentFinder, &LoopTool.getReplacements(),
                            &GeneratedDecls, &ReplacedVars, &AcceptedChanges,
                            &DeferredChanges, &RejectedChanges,
-                           MaxRisk, LFK_Array);
+                           Options().MaxRiskLevel, LFK_Array);
   Finder.addMatcher(makeArrayLoopMatcher(), &ArrayLoopFixer);
   LoopFixer IteratorLoopFixer(&ParentFinder, &LoopTool.getReplacements(),
                               &GeneratedDecls, &ReplacedVars,
                               &AcceptedChanges, &DeferredChanges,
                               &RejectedChanges,
-                              MaxRisk, LFK_Iterator);
+                              Options().MaxRiskLevel, LFK_Iterator);
   Finder.addMatcher(makeIteratorLoopMatcher(), &IteratorLoopFixer);
   LoopFixer PseudoarrrayLoopFixer(&ParentFinder, &LoopTool.getReplacements(),
                                   &GeneratedDecls, &ReplacedVars,
                                   &AcceptedChanges, &DeferredChanges,
                                   &RejectedChanges,
-                                  MaxRisk, LFK_PseudoArray);
+                                  Options().MaxRiskLevel, LFK_PseudoArray);
   Finder.addMatcher(makePseudoArrayLoopMatcher(), &PseudoarrrayLoopFixer);
 
   if (int result = LoopTool.run(
diff --git a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.h b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.h
index 25c6460af04..9dfff55be15 100644
--- a/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.h
+++ b/clang-tools-extra/cpp11-migrate/LoopConvert/LoopConvert.h
@@ -23,12 +23,11 @@
 /// for-loops where possible.
 class LoopConvertTransform : public Transform {
 public:
-  LoopConvertTransform(bool EnableTiming)
-      : Transform("LoopConvert", EnableTiming) {}
+  LoopConvertTransform(const TransformOptions &Options)
+      : Transform("LoopConvert", Options) {}
 
   /// \see Transform::run().
   virtual int apply(const FileContentsByPath &InputStates,
-                    RiskLevel MaxRiskLevel,
                     const clang::tooling::CompilationDatabase &Database,
                     const std::vector<std::string> &SourcePaths,
                     FileContentsByPath &ResultStates) LLVM_OVERRIDE;
diff --git a/clang-tools-extra/cpp11-migrate/UseAuto/UseAuto.cpp b/clang-tools-extra/cpp11-migrate/UseAuto/UseAuto.cpp
index a8e839d1c81..3c75246ea34 100644
--- a/clang-tools-extra/cpp11-migrate/UseAuto/UseAuto.cpp
+++ b/clang-tools-extra/cpp11-migrate/UseAuto/UseAuto.cpp
@@ -21,7 +21,6 @@ using namespace clang;
 using namespace clang::tooling;
 
 int UseAutoTransform::apply(const FileContentsByPath &InputStates,
-                            RiskLevel MaxRisk,
                             const clang::tooling::CompilationDatabase &Database,
                             const std::vector<std::string> &SourcePaths,
                             FileContentsByPath &ResultStates) {
@@ -36,9 +35,9 @@ int UseAutoTransform::apply(const FileContentsByPath &InputStates,
 
   MatchFinder Finder;
   IteratorReplacer ReplaceIterators(UseAutoTool.getReplacements(),
-                                    AcceptedChanges, MaxRisk);
+                                    AcceptedChanges, Options().MaxRiskLevel);
   NewReplacer ReplaceNew(UseAutoTool.getReplacements(), AcceptedChanges,
-                         MaxRisk);
+                         Options().MaxRiskLevel);
 
   Finder.addMatcher(makeIteratorDeclMatcher(), &ReplaceIterators);
   Finder.addMatcher(makeDeclWithNewMatcher(), &ReplaceNew);
diff --git a/clang-tools-extra/cpp11-migrate/UseAuto/UseAuto.h b/clang-tools-extra/cpp11-migrate/UseAuto/UseAuto.h
index 5e83d867a8e..b3bcc41b8e3 100644
--- a/clang-tools-extra/cpp11-migrate/UseAuto/UseAuto.h
+++ b/clang-tools-extra/cpp11-migrate/UseAuto/UseAuto.h
@@ -29,11 +29,10 @@
 /// p2 are not handled by this transform.
 class UseAutoTransform : public Transform {
 public:
-  UseAutoTransform(bool EnableTiming) : Transform("UseAuto", EnableTiming) {}
+  UseAutoTransform(const TransformOptions &Options) : Transform("UseAuto", Options) {}
 
   /// \see Transform::run().
   virtual int apply(const FileContentsByPath &InputStates,
-                    RiskLevel MaxRiskLEvel,
                     const clang::tooling::CompilationDatabase &Database,
                     const std::vector<std::string> &SourcePaths,
                     FileContentsByPath &ResultStates) LLVM_OVERRIDE;
diff --git a/clang-tools-extra/cpp11-migrate/UseNullptr/UseNullptr.cpp b/clang-tools-extra/cpp11-migrate/UseNullptr/UseNullptr.cpp
index 0f8b1a84f4b..58c6d438b61 100644
--- a/clang-tools-extra/cpp11-migrate/UseNullptr/UseNullptr.cpp
+++ b/clang-tools-extra/cpp11-migrate/UseNullptr/UseNullptr.cpp
@@ -26,7 +26,6 @@ using namespace clang::tooling;
 using namespace clang;
 
 int UseNullptrTransform::apply(const FileContentsByPath &InputStates,
-                               RiskLevel MaxRisk,
                                const CompilationDatabase &Database,
                                const std::vector<std::string> &SourcePaths,
                                FileContentsByPath &ResultStates) {
@@ -43,7 +42,7 @@ int UseNullptrTransform::apply(const FileContentsByPath &InputStates,
   MatchFinder Finder;
   NullptrFixer Fixer(UseNullptrTool.getReplacements(),
                      AcceptedChanges,
-                     MaxRisk);
+                     Options().MaxRiskLevel);
 
   Finder.addMatcher(makeCastSequenceMatcher(), &Fixer);
 
diff --git a/clang-tools-extra/cpp11-migrate/UseNullptr/UseNullptr.h b/clang-tools-extra/cpp11-migrate/UseNullptr/UseNullptr.h
index 49cdc45baa5..5b8328b942c 100644
--- a/clang-tools-extra/cpp11-migrate/UseNullptr/UseNullptr.h
+++ b/clang-tools-extra/cpp11-migrate/UseNullptr/UseNullptr.h
@@ -23,12 +23,11 @@
 /// C++11's nullptr keyword where possible.
 class UseNullptrTransform : public Transform {
 public:
-  UseNullptrTransform(bool EnableTiming)
-      : Transform("UseNullptr", EnableTiming) {}
+  UseNullptrTransform(const TransformOptions &Options)
+      : Transform("UseNullptr", Options) {}
 
   /// \see Transform::run().
   virtual int apply(const FileContentsByPath &InputStates,
-                    RiskLevel MaxRiskLEvel,
                     const clang::tooling::CompilationDatabase &Database,
                     const std::vector<std::string> &SourcePaths,
                     FileContentsByPath &ResultStates) LLVM_OVERRIDE;
diff --git a/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp b/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp
index 27ffafa603d..cd2e653fbc6 100644
--- a/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp
+++ b/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp
@@ -30,7 +30,9 @@
 namespace cl = llvm::cl;
 using namespace clang::tooling;
 
-static cl::opt<RiskLevel> MaxRiskLevel(
+TransformOptions GlobalOptions;
+
+static cl::opt<RiskLevel, /*ExternalStorage=*/true> MaxRiskLevel(
     "risk", cl::desc("Select a maximum risk level:"),
     cl::values(clEnumValN(RL_Safe, "safe", "Only safe transformations"),
                clEnumValN(RL_Reasonable, "reasonable",
@@ -40,6 +42,7 @@ static cl::opt<RiskLevel> MaxRiskLevel(
                           "Enable transformations that are likely to "
                           "change semantics"),
                clEnumValEnd),
+    cl::location(GlobalOptions.MaxRiskLevel),
     cl::init(RL_Reasonable));
 
 static cl::opt<bool> FinalSyntaxCheck(
@@ -108,8 +111,9 @@ int main(int argc, const char **argv) {
 
   // Since ExecutionTimeDirectoryName could be an empty string we compare
   // against the default value when the command line option is not specified.
-  bool EnableTiming = (TimingDirectoryName != NoTiming);
-  TransformManager.createSelectedTransforms(EnableTiming);
+  GlobalOptions.EnableTiming = (TimingDirectoryName != NoTiming);
+
+  TransformManager.createSelectedTransforms(GlobalOptions);
 
   if (TransformManager.begin() == TransformManager.end()) {
     llvm::errs() << "No selected transforms\n";
@@ -125,15 +129,14 @@ int main(int argc, const char **argv) {
   for (Transforms::const_iterator I = TransformManager.begin(),
                                   E = TransformManager.end();
        I != E; ++I) {
-    if ((*I)->apply(*InputFileStates, MaxRiskLevel,
-                    OptionsParser.getCompilations(),
+    if ((*I)->apply(*InputFileStates, OptionsParser.getCompilations(),
                     OptionsParser.getSourcePathList(), *OutputFileStates) !=
         0) {
       // FIXME: Improve ClangTool to not abort if just one file fails.
       return 1;
     }
 
-    if (EnableTiming)
+    if (GlobalOptions.EnableTiming)
       collectSourcePerfData(**I, PerfData);
 
     if (SummaryMode) {
@@ -184,7 +187,7 @@ int main(int argc, const char **argv) {
   }
 
   // Report execution times.
-  if (EnableTiming && !PerfData.empty()) {
+  if (GlobalOptions.EnableTiming && !PerfData.empty()) {
     std::string DirectoryName = TimingDirectoryName;
     // Use default directory name.
     if (DirectoryName.empty())
diff --git a/clang-tools-extra/unittests/cpp11-migrate/PerfSupportTest.cpp b/clang-tools-extra/unittests/cpp11-migrate/PerfSupportTest.cpp
index 8c362fd1a73..21f296068fe 100644
--- a/clang-tools-extra/unittests/cpp11-migrate/PerfSupportTest.cpp
+++ b/clang-tools-extra/unittests/cpp11-migrate/PerfSupportTest.cpp
@@ -6,10 +6,10 @@ using namespace clang;
 

 class TransformA : public Transform {

 public:

-  TransformA()

-      : Transform("TransformA", false) {}

+  TransformA(const TransformOptions &Options)

+      : Transform("TransformA", Options) {}

 

-  virtual int apply(const FileContentsByPath &, RiskLevel,

+  virtual int apply(const FileContentsByPath &,

                     const tooling::CompilationDatabase &,

                     const std::vector<std::string> &, FileContentsByPath &) {

     return 0;

@@ -22,10 +22,10 @@ public:
 

 class TransformB : public Transform {

 public:

-  TransformB()

-      : Transform("TransformB", false) {}

+  TransformB(const TransformOptions &Options)

+      : Transform("TransformB", Options) {}

 

-  virtual int apply(const FileContentsByPath &, RiskLevel,

+  virtual int apply(const FileContentsByPath &,

                     const tooling::CompilationDatabase &,

                     const std::vector<std::string> &, FileContentsByPath &) {

     return 0;

@@ -46,8 +46,9 @@ struct ExpectedResults {
 };

 

 TEST(PerfSupport, collectSourcePerfData) {

-  TransformA A;

-  TransformB B;

+  TransformOptions Options;

+  TransformA A(Options);

+  TransformB B(Options);

   

   // The actual durations don't matter. Below only their relative ordering is

   // tested to ensure times, labels, and sources all stay together properly.

diff --git a/clang-tools-extra/unittests/cpp11-migrate/TransformTest.cpp b/clang-tools-extra/unittests/cpp11-migrate/TransformTest.cpp
index 76e5e3a1f94..447feb4d969 100644
--- a/clang-tools-extra/unittests/cpp11-migrate/TransformTest.cpp
+++ b/clang-tools-extra/unittests/cpp11-migrate/TransformTest.cpp
@@ -9,11 +9,10 @@ using namespace clang;
 
 class DummyTransform : public Transform {
 public:
-  DummyTransform(llvm::StringRef Name, bool EnableTiming)
-      : Transform(Name, EnableTiming) {}
+  DummyTransform(llvm::StringRef Name, const TransformOptions &Options)
+      : Transform(Name, Options) {}
 
   virtual int apply(const FileContentsByPath &,
-                    RiskLevel ,
                     const tooling::CompilationDatabase &,
                     const std::vector<std::string> &,
                     FileContentsByPath &) { return 0; }
@@ -30,7 +29,9 @@ public:
 };
 
 TEST(Transform, Interface) {
-  DummyTransform T("my_transform", /*EnableTiming=*/false);
+  TransformOptions Options;
+  DummyTransform T("my_transform", Options);
+
   ASSERT_EQ("my_transform", T.getName());
   ASSERT_EQ(0u, T.getAcceptedChanges());
   ASSERT_EQ(0u, T.getRejectedChanges());
@@ -87,7 +88,9 @@ struct ConsumerFactory {
 };
 
 TEST(Transform, Timings) {
-  DummyTransform T("timing_transform", /*EnableTiming=*/true);
+  TransformOptions Options;
+  Options.EnableTiming = true;
+  DummyTransform T("timing_transform", Options);
 
   // All the path stuff is to make the test work independently of OS.