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
This commit is contained in:
Edwin Vane 2013-06-06 20:31:52 +00:00
parent fb5bddfd0a
commit 32e3553f83
15 changed files with 73 additions and 61 deletions

View File

@ -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) {

View File

@ -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;

View File

@ -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);

View File

@ -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;

View File

@ -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));
}
}
}

View File

@ -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.

View File

@ -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(

View File

@ -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;

View File

@ -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);

View File

@ -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;

View File

@ -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);

View File

@ -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;

View File

@ -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())

View File

@ -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.

View File

@ -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.