Hi arielbernal, tareqsiraj,
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.
http://llvm-reviews.chandlerc.com/D981
Files:
cpp11-migrate/AddOverride/AddOverride.cpp
cpp11-migrate/Core/Transform.cpp
cpp11-migrate/Core/Transform.h
cpp11-migrate/LoopConvert/LoopConvert.cpp
cpp11-migrate/UseAuto/UseAuto.cpp
cpp11-migrate/UseNullptr/UseNullptr.cpp
unittests/cpp11-migrate/TransformTest.cpp
Index: cpp11-migrate/AddOverride/AddOverride.cpp
===================================================================
--- cpp11-migrate/AddOverride/AddOverride.cpp
+++ cpp11-migrate/AddOverride/AddOverride.cpp
@@ -43,13 +43,14 @@
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;
}
Index: cpp11-migrate/Core/Transform.cpp
===================================================================
--- cpp11-migrate/Core/Transform.cpp
+++ cpp11-migrate/Core/Transform.cpp
@@ -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;
@@ -20,20 +21,18 @@
/// SourceFileCallbacks.
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 +43,21 @@
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 +113,17 @@
}
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 +139,6 @@
}
FrontendActionFactory *
-Transform::createActionFactory(MatchFinder &Finder,
- const FileOverrides &InputStates) {
- return new ActionFactory(Finder, InputStates, *this);
+Transform::createActionFactory(MatchFinder &Finder) {
+ return new ActionFactory(Finder, /*Owner=*/ *this);
}
Index: cpp11-migrate/Core/Transform.h
===================================================================
--- cpp11-migrate/Core/Transform.h
+++ cpp11-migrate/Core/Transform.h
@@ -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 @@
// Forward declarations
namespace clang {
+class CompilerInstance;
namespace tooling {
class CompilationDatabase;
+class FrontendActionFactory;
} // namespace tooling
namespace ast_matchers {
class MatchFinder;
@@ -112,21 +112,21 @@
/// \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 @@
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 @@
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;
Index: cpp11-migrate/LoopConvert/LoopConvert.cpp
===================================================================
--- cpp11-migrate/LoopConvert/LoopConvert.cpp
+++ cpp11-migrate/LoopConvert/LoopConvert.cpp
@@ -57,7 +57,9 @@
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;
}
Index: cpp11-migrate/UseAuto/UseAuto.cpp
===================================================================
--- cpp11-migrate/UseAuto/UseAuto.cpp
+++ cpp11-migrate/UseAuto/UseAuto.cpp
@@ -37,7 +37,9 @@
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;
}
Index: cpp11-migrate/UseNullptr/UseNullptr.cpp
===================================================================
--- cpp11-migrate/UseNullptr/UseNullptr.cpp
+++ cpp11-migrate/UseNullptr/UseNullptr.cpp
@@ -37,11 +37,11 @@
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;
}
Index: unittests/cpp11-migrate/TransformTest.cpp
===================================================================
--- unittests/cpp11-migrate/TransformTest.cpp
+++ unittests/cpp11-migrate/TransformTest.cpp
@@ -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 @@
void setDeferredChanges(unsigned Changes) {
Transform::setDeferredChanges(Changes);
}
+
+ void setOverrides(const FileOverrides &Overrides) {
+ Transform::setOverrides(Overrides);
+ }
+
};
TEST(Transform, Interface) {
@@ -58,9 +64,9 @@
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 @@
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 @@
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();
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
