[patch] FrontendActionFactory::create return by unique_ptr and some related cleanup
David Blaikie
dblaikie at gmail.com
Thu Sep 18 14:05:45 PDT 2014
Splitting some of this off from the "Own/produce FrontendActionFactories by
value, rather than new'd/unique_ptr ownership"
This is just to change the FrontendActionFactory::create to return by
unique_ptr.
But I did find a bunch of verbose and repetitious code (creating
FrontendActionFactories and FrontendActions) that could be greatly
simplified with some utilities and lambdas. This would be simpler if the
other FrontendActionFactor utility functions were returning by value too.
Anyway, bunch of things between this patch and the other:
1) Just the FrontendActionFactory::create -> unique_ptr change (propagates
to runToolOnCode* functions needing to take by unique_ptr too, as
previously discussed)
2) More FrontendActionFactory helpers with lambdas (these could use real
names - they just have placeholders at the moment)
3) Tool::run pass FrontendActionFactory by reference
4) Move existing FrontendActionFactory helpers to return by value (new* ->
make* rename, probably)
1 and 2 are in these patches (but can be separated and committed in either
order - or one/the other taken while dropping the other)
3 and 4 are in the other review, again - can be separated and
accepted/declined
But generally all 4 would work best together.
(1) without (2) is more churn on every caller - when I could just tidy up
most callers (most of the google-internal callers too -
https://github.com/google/souper/blob/master/lib/ClangTool/Actions.cpp#L76
is the only google-internal one that needs the "less trivial" flavor - all
the other internal FrontendActionFactory subclasses can be removed and
replaced with makeTrivialFrontendActionFactory).
(3) makes (1) and (2) easier - moving to more return-by-value means some
callers can't be easily migrated (see clang-modernize - which has a factory
producing unique_ptr<FrontendActionFactory> and callers that
"Tool.run(factory().get())" - until run takes a reference, that can't be
migrated to return by value without having to introduce extra vsariables at
the caller.
(4) would make those builder/factory/helper functions consistent with (2)
and avoid the need for dynamic allocation. Only one caller would be
inconvenienced by this, it's simpler for everyone else (and less subtle
code: "Tool.run(*newFrontendActionFactory(...))" - wait, does
newFrontendActionFactory return a raw owning pointer? Did this just leak?)
Sorry about going around and around with all this - hopefully this is a
reasonable summary of some of the interdependencies/benefits... I can try
to write something more structured/explicit if it'd be helpful. Happy to
split/apply these in whatever parts you see fit.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140918/26d43b2b/attachment.html>
-------------- next part --------------
commit e8717dc47917dcf67a6d2eea9994896e168db263
Author: David Blaikie <dblaikie at gmail.com>
Date: Wed Sep 17 14:16:46 2014 -0700
unique_ptrify FrontendActionFactory::create
diff --git clang-modernize/Core/Transform.cpp clang-modernize/Core/Transform.cpp
index b716fe5..65bf7d7 100644
--- clang-modernize/Core/Transform.cpp
+++ clang-modernize/Core/Transform.cpp
@@ -32,46 +32,32 @@ namespace {
using namespace tooling;
using namespace ast_matchers;
-/// \brief Custom FrontendActionFactory to produce FrontendActions that simply
-/// forward (Begin|End)SourceFileAction calls to a given Transform.
-class ActionFactory : public clang::tooling::FrontendActionFactory {
+/// \brief Custom FrontendActions that simply forward
+/// (Begin|End)SourceFileAction calls to a given Transform.
+class FactoryAdaptor : public ASTFrontendAction {
public:
- ActionFactory(MatchFinder &Finder, Transform &Owner)
- : Finder(Finder), Owner(Owner) {}
+ FactoryAdaptor(MatchFinder &Finder, Transform &Owner)
+ : Finder(Finder), Owner(Owner) {}
- virtual FrontendAction *create() override {
- return new FactoryAdaptor(Finder, Owner);
+ std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &,
+ StringRef) {
+ return Finder.newASTConsumer();
}
-private:
- class FactoryAdaptor : public ASTFrontendAction {
- public:
- FactoryAdaptor(MatchFinder &Finder, Transform &Owner)
- : Finder(Finder), Owner(Owner) {}
-
- std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &,
- StringRef) {
- return Finder.newASTConsumer();
- }
-
- virtual bool BeginSourceFileAction(CompilerInstance &CI,
- StringRef Filename) override {
- if (!ASTFrontendAction::BeginSourceFileAction(CI, Filename))
- return false;
-
- return Owner.handleBeginSource(CI, Filename);
- }
-
- virtual void EndSourceFileAction() override {
- Owner.handleEndSource();
- return ASTFrontendAction::EndSourceFileAction();
- }
-
- private:
- MatchFinder &Finder;
- Transform &Owner;
- };
+ virtual bool BeginSourceFileAction(CompilerInstance &CI,
+ StringRef Filename) override {
+ if (!ASTFrontendAction::BeginSourceFileAction(CI, Filename))
+ return false;
+
+ return Owner.handleBeginSource(CI, Filename);
+ }
+ virtual void EndSourceFileAction() override {
+ Owner.handleEndSource();
+ return ASTFrontendAction::EndSourceFileAction();
+ }
+
+private:
MatchFinder &Finder;
Transform &Owner;
};
@@ -131,7 +117,9 @@ Transform::addReplacementForCurrentTU(const clang::tooling::Replacement &R) {
std::unique_ptr<FrontendActionFactory>
Transform::createActionFactory(MatchFinder &Finder) {
- return llvm::make_unique<ActionFactory>(Finder, /*Owner=*/*this);
+ return newT(makeLessTrivialFrontendActionFactory([&]() {
+ return llvm::make_unique<FactoryAdaptor>(Finder, /*Owner=*/*this);
+ }));
}
Version Version::getFromString(llvm::StringRef VersionStr) {
diff --git clang-tidy/ClangTidy.cpp clang-tidy/ClangTidy.cpp
index 0712f69..2b5ee35 100644
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -363,28 +363,11 @@ runClangTidy(std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
Tool.setDiagnosticConsumer(&DiagConsumer);
- class ActionFactory : public FrontendActionFactory {
- public:
- ActionFactory(ClangTidyContext &Context) : ConsumerFactory(Context) {}
- FrontendAction *create() override { return new Action(&ConsumerFactory); }
-
- private:
- class Action : public ASTFrontendAction {
- public:
- Action(ClangTidyASTConsumerFactory *Factory) : Factory(Factory) {}
- std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
- StringRef File) override {
- return Factory->CreateASTConsumer(Compiler, File);
- }
-
- private:
- ClangTidyASTConsumerFactory *Factory;
- };
-
- ClangTidyASTConsumerFactory ConsumerFactory;
- };
-
- ActionFactory Factory(Context);
+ ClangTidyASTConsumerFactory ConsumerFactory(Context);
+ auto Factory = makeTrivialFrontendActionFactory(
+ [&](CompilerInstance &Compiler, StringRef File) {
+ return ConsumerFactory.CreateASTConsumer(Compiler, File);
+ });
Tool.run(&Factory);
*Errors = Context.getErrors();
return Context.getStats();
diff --git modularize/Modularize.cpp modularize/Modularize.cpp
index 83c5547..65094a2 100644
--- modularize/Modularize.cpp
+++ modularize/Modularize.cpp
@@ -643,45 +643,6 @@ private:
int &HadErrors;
};
-class CollectEntitiesAction : public SyntaxOnlyAction {
-public:
- CollectEntitiesAction(EntityMap &Entities,
- PreprocessorTracker &preprocessorTracker,
- int &HadErrors)
- : Entities(Entities), PPTracker(preprocessorTracker),
- HadErrors(HadErrors) {}
-
-protected:
- std::unique_ptr<clang::ASTConsumer>
- CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
- return llvm::make_unique<CollectEntitiesConsumer>(
- Entities, PPTracker, CI.getPreprocessor(), InFile, HadErrors);
- }
-
-private:
- EntityMap &Entities;
- PreprocessorTracker &PPTracker;
- int &HadErrors;
-};
-
-class ModularizeFrontendActionFactory : public FrontendActionFactory {
-public:
- ModularizeFrontendActionFactory(EntityMap &Entities,
- PreprocessorTracker &preprocessorTracker,
- int &HadErrors)
- : Entities(Entities), PPTracker(preprocessorTracker),
- HadErrors(HadErrors) {}
-
- virtual CollectEntitiesAction *create() {
- return new CollectEntitiesAction(Entities, PPTracker, HadErrors);
- }
-
-private:
- EntityMap &Entities;
- PreprocessorTracker &PPTracker;
- int &HadErrors;
-};
-
int main(int Argc, const char **Argv) {
// Save program name for error messages.
@@ -736,7 +697,11 @@ int main(int Argc, const char **Argv) {
ClangTool Tool(*Compilations, Headers);
Tool.appendArgumentsAdjuster(new AddDependenciesAdjuster(Dependencies));
int HadErrors = 0;
- ModularizeFrontendActionFactory Factory(Entities, *PPTracker, HadErrors);
+ auto Factory = makeTrivialFrontendActionFactory(
+ [&](CompilerInstance &CI, StringRef InFile) {
+ return llvm::make_unique<CollectEntitiesConsumer>(
+ Entities, *PPTracker, CI.getPreprocessor(), InFile, HadErrors);
+ });
HadErrors |= Tool.run(&Factory);
// Create a place to save duplicate entity locations, separate bins per kind.
diff --git module-map-checker/ModuleMapChecker.cpp module-map-checker/ModuleMapChecker.cpp
index e1ae764..b722f81 100644
--- module-map-checker/ModuleMapChecker.cpp
+++ module-map-checker/ModuleMapChecker.cpp
@@ -164,43 +164,6 @@ private:
// Frontend action stuff:
-// Consumer is responsible for setting up the callbacks.
-class ModuleMapCheckerConsumer : public ASTConsumer {
-public:
- ModuleMapCheckerConsumer(ModuleMapChecker &Checker, Preprocessor &PP) {
- // PP takes ownership.
- PP.addPPCallbacks(llvm::make_unique<ModuleMapCheckerCallbacks>(Checker));
- }
-};
-
-class ModuleMapCheckerAction : public SyntaxOnlyAction {
-public:
- ModuleMapCheckerAction(ModuleMapChecker &Checker) : Checker(Checker) {}
-
-protected:
- std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
- StringRef InFile) override {
- return llvm::make_unique<ModuleMapCheckerConsumer>(Checker,
- CI.getPreprocessor());
- }
-
-private:
- ModuleMapChecker &Checker;
-};
-
-class ModuleMapCheckerFrontendActionFactory : public FrontendActionFactory {
-public:
- ModuleMapCheckerFrontendActionFactory(ModuleMapChecker &Checker)
- : Checker(Checker) {}
-
- virtual ModuleMapCheckerAction *create() {
- return new ModuleMapCheckerAction(Checker);
- }
-
-private:
- ModuleMapChecker &Checker;
-};
-
// ModuleMapChecker class implementation.
// Constructor.
@@ -404,7 +367,13 @@ ModuleMapChecker::collectUmbrellaHeaderHeaders(StringRef UmbrellaHeaderName) {
// Create the tool and run the compilation.
ClangTool Tool(*Compilations, HeaderPath);
- int HadErrors = Tool.run(new ModuleMapCheckerFrontendActionFactory(*this));
+ auto Factory = makeTrivialFrontendActionFactory(
+ [&](CompilerInstance &CI, StringRef InFile) {
+ CI.getPreprocessor().addPPCallbacks(
+ llvm::make_unique<ModuleMapCheckerCallbacks>(*this));
+ return llvm::make_unique<ASTConsumer>();
+ });
+ int HadErrors = Tool.run(&Factory);
// If we had errors, exit early.
return HadErrors ? false : true;
diff --git pp-trace/PPTrace.cpp pp-trace/PPTrace.cpp
index aae6215..51974b4 100644
--- pp-trace/PPTrace.cpp
+++ pp-trace/PPTrace.cpp
@@ -101,52 +101,6 @@ cl::list<std::string>
CC1Arguments(cl::ConsumeAfter,
cl::desc("<arguments to be passed to front end>..."));
-// Frontend action stuff:
-
-// Consumer is responsible for setting up the callbacks.
-class PPTraceConsumer : public ASTConsumer {
-public:
- PPTraceConsumer(SmallSet<std::string, 4> &Ignore,
- std::vector<CallbackCall> &CallbackCalls, Preprocessor &PP) {
- // PP takes ownership.
- PP.addPPCallbacks(llvm::make_unique<PPCallbacksTracker>(Ignore,
- CallbackCalls, PP));
- }
-};
-
-class PPTraceAction : public SyntaxOnlyAction {
-public:
- PPTraceAction(SmallSet<std::string, 4> &Ignore,
- std::vector<CallbackCall> &CallbackCalls)
- : Ignore(Ignore), CallbackCalls(CallbackCalls) {}
-
-protected:
- std::unique_ptr<clang::ASTConsumer>
- CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
- return llvm::make_unique<PPTraceConsumer>(Ignore, CallbackCalls,
- CI.getPreprocessor());
- }
-
-private:
- SmallSet<std::string, 4> &Ignore;
- std::vector<CallbackCall> &CallbackCalls;
-};
-
-class PPTraceFrontendActionFactory : public FrontendActionFactory {
-public:
- PPTraceFrontendActionFactory(SmallSet<std::string, 4> &Ignore,
- std::vector<CallbackCall> &CallbackCalls)
- : Ignore(Ignore), CallbackCalls(CallbackCalls) {}
-
- virtual PPTraceAction *create() {
- return new PPTraceAction(Ignore, CallbackCalls);
- }
-
-private:
- SmallSet<std::string, 4> &Ignore;
- std::vector<CallbackCall> &CallbackCalls;
-};
-
// Output the trace given its data structure and a stream.
int outputPPTrace(std::vector<CallbackCall> &CallbackCalls,
llvm::raw_ostream &OS) {
@@ -201,7 +155,13 @@ int main(int Argc, const char **Argv) {
// Create the tool and run the compilation.
ClangTool Tool(*Compilations, SourcePaths);
- PPTraceFrontendActionFactory Factory(Ignore, CallbackCalls);
+ auto Factory = makeTrivialFrontendActionFactory(
+ [&](CompilerInstance &CI, StringRef InFile) {
+ auto &PP = CI.getPreprocessor();
+ PP.addPPCallbacks(
+ llvm::make_unique<PPCallbacksTracker>(Ignore, CallbackCalls, PP));
+ return llvm::make_unique<ASTConsumer>();
+ });
int HadErrors = Tool.run(&Factory);
// If we had errors, exit early.
diff --git unittests/clang-modernize/IncludeDirectivesTest.cpp unittests/clang-modernize/IncludeDirectivesTest.cpp
index b135288..f217597 100644
--- unittests/clang-modernize/IncludeDirectivesTest.cpp
+++ unittests/clang-modernize/IncludeDirectivesTest.cpp
@@ -19,7 +19,8 @@ using namespace clang;
/// \brief A convenience method around \c tooling::runToolOnCodeWithArgs() that
/// adds the current directory to the include search paths.
-static void applyActionOnCode(FrontendAction *ToolAction, StringRef Code) {
+static void applyActionOnCode(std::unique_ptr<FrontendAction> ToolAction,
+ StringRef Code) {
SmallString<128> CurrentDir;
ASSERT_FALSE(llvm::sys::fs::current_path(CurrentDir));
@@ -33,8 +34,8 @@ static void applyActionOnCode(FrontendAction *ToolAction, StringRef Code) {
SmallString<128> InputFile(CurrentDir);
sys::path::append(InputFile, "input.cc");
- ASSERT_TRUE(
- tooling::runToolOnCodeWithArgs(ToolAction, Code, Args, InputFile.str()));
+ ASSERT_TRUE(tooling::runToolOnCodeWithArgs(std::move(ToolAction), Code, Args,
+ InputFile.str()));
}
namespace {
@@ -115,7 +116,8 @@ private:
std::string addIncludeInCode(StringRef Include, StringRef Code) {
tooling::Replacements Replaces;
- applyActionOnCode(new TestAddIncludeAction(Include, Replaces), Code);
+ applyActionOnCode(llvm::make_unique<TestAddIncludeAction>(Include, Replaces),
+ Code);
if (::testing::Test::HasFailure())
return "<<unexpected error from applyActionOnCode()>>";
@@ -236,11 +238,12 @@ TEST(IncludeDirectivesTest, ignoreHeadersMeantForMultipleInclusion) {
}
namespace {
-TestAddIncludeAction *makeIndirectTestsAction(const char *HeaderToModify,
- tooling::Replacements &Replaces) {
+std::unique_ptr<TestAddIncludeAction>
+makeIndirectTestsAction(const char *HeaderToModify,
+ tooling::Replacements &Replaces) {
StringRef IncludeToAdd = "c.h";
- TestAddIncludeAction *TestAction =
- new TestAddIncludeAction(IncludeToAdd, Replaces, HeaderToModify);
+ auto TestAction = llvm::make_unique<TestAddIncludeAction>(
+ IncludeToAdd, Replaces, HeaderToModify);
TestAction->mapVirtualHeader("c.h", "#pragma once\n");
TestAction->mapVirtualHeader("a.h", "#pragma once\n"
"#include <c.h>\n");
@@ -261,16 +264,16 @@ TEST(IncludeDirectivesTest, indirectIncludes) {
// a.h already includes c.h
{
- FrontendAction *Action = makeIndirectTestsAction("a.h", Replaces);
- ASSERT_NO_FATAL_FAILURE(applyActionOnCode(Action, Code));
+ auto Action = makeIndirectTestsAction("a.h", Replaces);
+ ASSERT_NO_FATAL_FAILURE(applyActionOnCode(std::move(Action), Code));
EXPECT_EQ(unsigned(0), Replaces.size());
}
// c.h is included before b.h but b.h doesn't include c.h directly, so check
// that it will be inserted.
{
- FrontendAction *Action = makeIndirectTestsAction("b.h", Replaces);
- ASSERT_NO_FATAL_FAILURE(applyActionOnCode(Action, Code));
+ auto Action = makeIndirectTestsAction("b.h", Replaces);
+ ASSERT_NO_FATAL_FAILURE(applyActionOnCode(std::move(Action), Code));
EXPECT_EQ("#include <c.h>\n\n\n",
tooling::applyAllReplacements("\n", Replaces));
}
@@ -281,11 +284,11 @@ static std::string addIncludeInGuardedHeader(StringRef IncludeToAdd,
StringRef GuardedHeaderCode) {
const char *GuardedHeaderName = "guarded.h";
tooling::Replacements Replaces;
- TestAddIncludeAction *TestAction =
- new TestAddIncludeAction(IncludeToAdd, Replaces, GuardedHeaderName);
+ auto TestAction = llvm::make_unique<TestAddIncludeAction>(
+ IncludeToAdd, Replaces, GuardedHeaderName);
TestAction->mapVirtualHeader(GuardedHeaderName, GuardedHeaderCode);
- applyActionOnCode(TestAction, "#include <guarded.h>\n");
+ applyActionOnCode(std::move(TestAction), "#include <guarded.h>\n");
if (::testing::Test::HasFailure())
return "<<unexpected error from applyActionOnCode()>>";
diff --git unittests/clang-tidy/ClangTidyTest.h unittests/clang-tidy/ClangTidyTest.h
index 181e6c2..3d1dce1 100644
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -53,8 +53,9 @@ std::string runCheckOnCode(StringRef Code,
std::vector<std::string> ArgCXX11(1, "-std=c++11");
ArgCXX11.insert(ArgCXX11.end(), ExtraArgs.begin(), ExtraArgs.end());
- if (!tooling::runToolOnCodeWithArgs(new TestPPAction(Check, &Context), Code,
- ArgCXX11, Filename))
+ if (!tooling::runToolOnCodeWithArgs(
+ llvm::make_unique<TestPPAction>(Check, &Context), Code, ArgCXX11,
+ Filename))
return "";
ast_matchers::MatchFinder Finder;
Check.registerMatchers(&Finder);
-------------- next part --------------
commit 63956c9cf005356c9cbde87308d9d504182c1df7
Author: David Blaikie <dblaikie at gmail.com>
Date: Thu Sep 18 13:46:31 2014 -0700
unique_ptrify FrontendActionFactory::create
diff --git include/clang/Tooling/Tooling.h include/clang/Tooling/Tooling.h
index 6f0d579..9cecab5 100644
--- include/clang/Tooling/Tooling.h
+++ include/clang/Tooling/Tooling.h
@@ -36,6 +36,7 @@
#include "clang/Basic/LLVM.h"
#include "clang/Driver/Util.h"
#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
#include "clang/Tooling/ArgumentsAdjusters.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/StringMap.h"
@@ -87,7 +88,7 @@ public:
/// \brief Returns a new clang::FrontendAction.
///
/// The caller takes ownership of the returned action.
- virtual clang::FrontendAction *create() = 0;
+ virtual std::unique_ptr<clang::FrontendAction> create() = 0;
};
/// \brief Returns a new FrontendActionFactory for a given type.
@@ -133,6 +134,48 @@ template <typename FactoryT>
inline std::unique_ptr<FrontendActionFactory> newFrontendActionFactory(
FactoryT *ConsumerFactory, SourceFileCallbacks *Callbacks = nullptr);
+template<typename Functor>
+class DelegatingFrontendActionFactory : public FrontendActionFactory {
+ Functor F;
+public:
+ explicit DelegatingFrontendActionFactory(Functor F) : F(std::move(F)) {
+ }
+ std::unique_ptr<FrontendAction> create() override {
+ return F();
+ }
+};
+
+template<typename Functor>
+class DoubleDelegatingFrontendActionFactory: public FrontendActionFactory {
+ Functor F;
+public:
+ explicit DoubleDelegatingFrontendActionFactory(Functor F) : F(std::move(F)) {}
+ std::unique_ptr<FrontendAction> create() override {
+ struct FrontendAction : ASTFrontendAction {
+ Functor F;
+ FrontendAction(Functor F) : F(std::move(F)) {
+ }
+ std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+ StringRef InFile) {
+ return F(CI, InFile);
+ }
+ };
+ return llvm::make_unique<FrontendAction>(F);
+ }
+};
+
+template<typename Functor>
+DelegatingFrontendActionFactory<Functor>
+makeLessTrivialFrontendActionFactory(Functor F) {
+ return DelegatingFrontendActionFactory<Functor>(std::move(F));
+}
+
+template <typename Functor>
+DoubleDelegatingFrontendActionFactory<Functor>
+makeTrivialFrontendActionFactory(Functor F) {
+ return DoubleDelegatingFrontendActionFactory<Functor>(std::move(F));
+}
+
/// \brief Runs (and deletes) the tool on 'Code' with the -fsyntax-only flag.
///
/// \param ToolAction The action to run over the code.
@@ -140,8 +183,8 @@ inline std::unique_ptr<FrontendActionFactory> newFrontendActionFactory(
/// \param FileName The file name which 'Code' will be mapped as.
///
/// \return - True if 'ToolAction' was successfully executed.
-bool runToolOnCode(clang::FrontendAction *ToolAction, const Twine &Code,
- const Twine &FileName = "input.cc");
+bool runToolOnCode(std::unique_ptr<clang::FrontendAction> ToolAction,
+ const Twine &Code, const Twine &FileName = "input.cc");
/// \brief Runs (and deletes) the tool on 'Code' with the -fsyntax-only flag and
/// with additional other flags.
@@ -152,7 +195,8 @@ bool runToolOnCode(clang::FrontendAction *ToolAction, const Twine &Code,
/// \param FileName The file name which 'Code' will be mapped as.
///
/// \return - True if 'ToolAction' was successfully executed.
-bool runToolOnCodeWithArgs(clang::FrontendAction *ToolAction, const Twine &Code,
+bool runToolOnCodeWithArgs(std::unique_ptr<clang::FrontendAction> ToolAction,
+ const Twine &Code,
const std::vector<std::string> &Args,
const Twine &FileName = "input.cc");
@@ -179,7 +223,7 @@ buildASTFromCodeWithArgs(const Twine &Code,
/// \brief Utility to run a FrontendAction in a single clang invocation.
class ToolInvocation {
- public:
+public:
/// \brief Create a tool invocation.
///
/// \param CommandLine The command line arguments to clang. Note that clang
@@ -189,8 +233,8 @@ class ToolInvocation {
/// \param FAction The action to be executed. Class takes ownership.
/// \param Files The FileManager used for the execution. Class does not take
/// ownership.
- ToolInvocation(std::vector<std::string> CommandLine, FrontendAction *FAction,
- FileManager *Files);
+ ToolInvocation(std::vector<std::string> CommandLine,
+ std::unique_ptr<FrontendAction> FAction, FileManager *Files);
/// \brief Create a tool invocation.
///
@@ -305,67 +349,54 @@ class ClangTool {
DiagnosticConsumer *DiagConsumer;
};
+template <typename T> std::unique_ptr<T> newT(T &&t) {
+ return llvm::make_unique<T>(std::forward<T>(t));
+}
+
template <typename T>
std::unique_ptr<FrontendActionFactory> newFrontendActionFactory() {
- class SimpleFrontendActionFactory : public FrontendActionFactory {
- public:
- clang::FrontendAction *create() override { return new T; }
- };
-
- return std::unique_ptr<FrontendActionFactory>(
- new SimpleFrontendActionFactory);
+ return newT(makeLessTrivialFrontendActionFactory([]() {
+ return llvm::make_unique<T>(); }));
}
template <typename FactoryT>
inline std::unique_ptr<FrontendActionFactory> newFrontendActionFactory(
FactoryT *ConsumerFactory, SourceFileCallbacks *Callbacks) {
- class FrontendActionFactoryAdapter : public FrontendActionFactory {
+ class ConsumerFactoryAdaptor : public clang::ASTFrontendAction {
public:
- explicit FrontendActionFactoryAdapter(FactoryT *ConsumerFactory,
- SourceFileCallbacks *Callbacks)
- : ConsumerFactory(ConsumerFactory), Callbacks(Callbacks) {}
-
- clang::FrontendAction *create() override {
- return new ConsumerFactoryAdaptor(ConsumerFactory, Callbacks);
- }
-
- private:
- class ConsumerFactoryAdaptor : public clang::ASTFrontendAction {
- public:
- ConsumerFactoryAdaptor(FactoryT *ConsumerFactory,
- SourceFileCallbacks *Callbacks)
+ ConsumerFactoryAdaptor(FactoryT *ConsumerFactory,
+ SourceFileCallbacks *Callbacks)
: ConsumerFactory(ConsumerFactory), Callbacks(Callbacks) {}
- std::unique_ptr<clang::ASTConsumer>
- CreateASTConsumer(clang::CompilerInstance &, StringRef) override {
- return ConsumerFactory->newASTConsumer();
- }
+ std::unique_ptr<clang::ASTConsumer>
+ CreateASTConsumer(clang::CompilerInstance &, StringRef) override {
+ return ConsumerFactory->newASTConsumer();
+ }
- protected:
- bool BeginSourceFileAction(CompilerInstance &CI,
- StringRef Filename) override {
- if (!clang::ASTFrontendAction::BeginSourceFileAction(CI, Filename))
- return false;
- if (Callbacks)
- return Callbacks->handleBeginSource(CI, Filename);
- return true;
- }
- void EndSourceFileAction() override {
- if (Callbacks)
- Callbacks->handleEndSource();
- clang::ASTFrontendAction::EndSourceFileAction();
- }
+ protected:
+ bool BeginSourceFileAction(CompilerInstance &CI,
+ StringRef Filename) override {
+ if (!clang::ASTFrontendAction::BeginSourceFileAction(CI, Filename))
+ return false;
+ if (Callbacks)
+ return Callbacks->handleBeginSource(CI, Filename);
+ return true;
+ }
+ void EndSourceFileAction() override {
+ if (Callbacks)
+ Callbacks->handleEndSource();
+ clang::ASTFrontendAction::EndSourceFileAction();
+ }
- private:
- FactoryT *ConsumerFactory;
- SourceFileCallbacks *Callbacks;
- };
+ private:
FactoryT *ConsumerFactory;
SourceFileCallbacks *Callbacks;
};
- return std::unique_ptr<FrontendActionFactory>(
- new FrontendActionFactoryAdapter(ConsumerFactory, Callbacks));
+ return newT(makeLessTrivialFrontendActionFactory([=]() {
+ return llvm::make_unique<ConsumerFactoryAdaptor>(ConsumerFactory,
+ Callbacks);
+ }));
}
/// \brief Returns the absolute path of \c File, by prepending it with
diff --git lib/Tooling/Tooling.cpp lib/Tooling/Tooling.cpp
index 230f73c..c8c42e8 100644
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -104,10 +104,10 @@ static clang::CompilerInvocation *newInvocation(
return Invocation;
}
-bool runToolOnCode(clang::FrontendAction *ToolAction, const Twine &Code,
- const Twine &FileName) {
- return runToolOnCodeWithArgs(
- ToolAction, Code, std::vector<std::string>(), FileName);
+bool runToolOnCode(std::unique_ptr<clang::FrontendAction> ToolAction,
+ const Twine &Code, const Twine &FileName) {
+ return runToolOnCodeWithArgs(std::move(ToolAction), Code,
+ std::vector<std::string>(), FileName);
}
static std::vector<std::string>
@@ -121,15 +121,16 @@ getSyntaxOnlyToolArgs(const std::vector<std::string> &ExtraArgs,
return Args;
}
-bool runToolOnCodeWithArgs(clang::FrontendAction *ToolAction, const Twine &Code,
+bool runToolOnCodeWithArgs(std::unique_ptr<clang::FrontendAction> ToolAction,
+ const Twine &Code,
const std::vector<std::string> &Args,
const Twine &FileName) {
SmallString<16> FileNameStorage;
StringRef FileNameRef = FileName.toNullTerminatedStringRef(FileNameStorage);
llvm::IntrusiveRefCntPtr<FileManager> Files(
new FileManager(FileSystemOptions()));
- ToolInvocation Invocation(getSyntaxOnlyToolArgs(Args, FileNameRef), ToolAction,
- Files.get());
+ ToolInvocation Invocation(getSyntaxOnlyToolArgs(Args, FileNameRef),
+ std::move(ToolAction), Files.get());
SmallString<1024> CodeStorage;
Invocation.mapVirtualFile(FileNameRef,
@@ -155,12 +156,12 @@ std::string getAbsolutePath(StringRef File) {
namespace {
class SingleFrontendActionFactory : public FrontendActionFactory {
- FrontendAction *Action;
+ std::unique_ptr<FrontendAction> Action;
public:
- SingleFrontendActionFactory(FrontendAction *Action) : Action(Action) {}
+ SingleFrontendActionFactory(std::unique_ptr<FrontendAction> Action) : Action(std::move(Action)) {}
- FrontendAction *create() override { return Action; }
+ std::unique_ptr<FrontendAction> create() override { return std::move(Action); }
};
}
@@ -174,9 +175,9 @@ ToolInvocation::ToolInvocation(std::vector<std::string> CommandLine,
DiagConsumer(nullptr) {}
ToolInvocation::ToolInvocation(std::vector<std::string> CommandLine,
- FrontendAction *FAction, FileManager *Files)
+ std::unique_ptr<FrontendAction> FAction, FileManager *Files)
: CommandLine(std::move(CommandLine)),
- Action(new SingleFrontendActionFactory(FAction)),
+ Action(new SingleFrontendActionFactory(std::move(FAction))),
OwnsAction(true),
Files(Files),
DiagConsumer(nullptr) {}
@@ -256,7 +257,7 @@ bool FrontendActionFactory::runInvocation(CompilerInvocation *Invocation,
// The FrontendAction can have lifetime requirements for Compiler or its
// members, and we need to ensure it's deleted earlier than Compiler. So we
// pass it to an std::unique_ptr declared after the Compiler variable.
- std::unique_ptr<FrontendAction> ScopedToolAction(create());
+ std::unique_ptr<FrontendAction> ScopedToolAction = create();
// Create the compiler's actual diagnostics engine.
Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
diff --git unittests/AST/EvaluateAsRValueTest.cpp unittests/AST/EvaluateAsRValueTest.cpp
index b5f9b32..44c9562 100644
--- unittests/AST/EvaluateAsRValueTest.cpp
+++ unittests/AST/EvaluateAsRValueTest.cpp
@@ -92,7 +92,7 @@ TEST(EvaluateAsRValue, FailsGracefullyForUnknownTypes) {
std::vector<std::string> Args(1, Mode);
Args.push_back("-fno-delayed-template-parsing");
ASSERT_TRUE(runToolOnCodeWithArgs(
- new EvaluateConstantInitializersAction(),
+ llvm::make_unique<EvaluateConstantInitializersAction>(),
"template <typename T>"
"struct vector {"
" explicit vector(int size);"
diff --git unittests/Sema/ExternalSemaSourceTest.cpp unittests/Sema/ExternalSemaSourceTest.cpp
index 4291b76..c6cc7a1 100644
--- unittests/Sema/ExternalSemaSourceTest.cpp
+++ unittests/Sema/ExternalSemaSourceTest.cpp
@@ -184,7 +184,7 @@ TEST(ExternalSemaSource, SanityCheck) {
Installer->PushWatcher(&Watcher);
std::vector<std::string> Args(1, "-std=c++11");
ASSERT_TRUE(clang::tooling::runToolOnCodeWithArgs(
- Installer.release(), "namespace AAA { } using namespace AAB;", Args));
+ std::move(Installer), "namespace AAA { } using namespace AAB;", Args));
ASSERT_EQ(0, Watcher.SeenCount);
}
@@ -199,7 +199,7 @@ TEST(ExternalSemaSource, ExternalTypoCorrectionPrioritized) {
Installer->PushWatcher(&Watcher);
std::vector<std::string> Args(1, "-std=c++11");
ASSERT_TRUE(clang::tooling::runToolOnCodeWithArgs(
- Installer.release(), "namespace AAA { } using namespace AAB;", Args));
+ std::move(Installer), "namespace AAA { } using namespace AAB;", Args));
ASSERT_LE(0, Provider.CallCount);
ASSERT_EQ(1, Watcher.SeenCount);
}
@@ -219,7 +219,7 @@ TEST(ExternalSemaSource, ExternalTypoCorrectionOrdering) {
Installer->PushWatcher(&Watcher);
std::vector<std::string> Args(1, "-std=c++11");
ASSERT_TRUE(clang::tooling::runToolOnCodeWithArgs(
- Installer.release(), "namespace AAA { } using namespace AAB;", Args));
+ std::move(Installer), "namespace AAA { } using namespace AAB;", Args));
ASSERT_LE(1, First.CallCount);
ASSERT_LE(1, Second.CallCount);
ASSERT_EQ(0, Third.CallCount);
@@ -237,7 +237,7 @@ TEST(ExternalSemaSource, TryOtherTacticsBeforeDiagnosing) {
// This code hits the class template specialization/class member of a class
// template specialization checks in Sema::RequireCompleteTypeImpl.
ASSERT_TRUE(clang::tooling::runToolOnCodeWithArgs(
- Installer.release(),
+ std::move(Installer),
"template <typename T> struct S { class C { }; }; S<char>::C SCInst;",
Args));
ASSERT_EQ(0, Diagnoser.CallCount);
@@ -256,7 +256,7 @@ TEST(ExternalSemaSource, FirstDiagnoserTaken) {
Installer->PushSource(&Third);
std::vector<std::string> Args(1, "-std=c++11");
ASSERT_FALSE(clang::tooling::runToolOnCodeWithArgs(
- Installer.release(), "class Incomplete; Incomplete IncompleteInstance;",
+ std::move(Installer), "class Incomplete; Incomplete IncompleteInstance;",
Args));
ASSERT_EQ(1, First.CallCount);
ASSERT_EQ(1, Second.CallCount);
diff --git unittests/Tooling/CommentHandlerTest.cpp unittests/Tooling/CommentHandlerTest.cpp
index 117dfc3..7b8e465 100644
--- unittests/Tooling/CommentHandlerTest.cpp
+++ unittests/Tooling/CommentHandlerTest.cpp
@@ -56,8 +56,8 @@ public:
CommentVerifier GetVerifier();
protected:
- virtual ASTFrontendAction* CreateTestAction() {
- return new CommentHandlerAction(this);
+ std::unique_ptr<ASTFrontendAction> CreateTestAction() override {
+ return llvm::make_unique<CommentHandlerAction>(this);
}
private:
diff --git unittests/Tooling/RefactoringTest.cpp unittests/Tooling/RefactoringTest.cpp
index 73c53aa..f46af11 100644
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -276,7 +276,7 @@ template <typename T>
class TestVisitor : public clang::RecursiveASTVisitor<T> {
public:
bool runOver(StringRef Code) {
- return runToolOnCode(new TestAction(this), Code);
+ return runToolOnCode(llvm::make_unique<TestAction>(this), Code);
}
protected:
diff --git unittests/Tooling/TestVisitor.h unittests/Tooling/TestVisitor.h
index 7680d07..9711191 100644
--- unittests/Tooling/TestVisitor.h
+++ unittests/Tooling/TestVisitor.h
@@ -74,8 +74,8 @@ public:
}
protected:
- virtual ASTFrontendAction* CreateTestAction() {
- return new TestAction(this);
+ virtual std::unique_ptr<ASTFrontendAction> CreateTestAction() {
+ return llvm::make_unique<TestAction>(this);
}
class FindConsumer : public ASTConsumer {
diff --git unittests/Tooling/ToolingTest.cpp unittests/Tooling/ToolingTest.cpp
index 85ab942..94448c9 100644
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -29,14 +29,13 @@ namespace {
/// works with single translation unit compilations.
class TestAction : public clang::ASTFrontendAction {
public:
- /// Takes ownership of TestConsumer.
explicit TestAction(std::unique_ptr<clang::ASTConsumer> TestConsumer)
: TestConsumer(std::move(TestConsumer)) {}
protected:
- virtual std::unique_ptr<clang::ASTConsumer>
- CreateASTConsumer(clang::CompilerInstance &compiler, StringRef dummy) {
- /// TestConsumer will be deleted by the framework calling us.
+ std::unique_ptr<clang::ASTConsumer>
+ CreateASTConsumer(clang::CompilerInstance &compiler,
+ StringRef dummy) override {
return std::move(TestConsumer);
}
@@ -60,7 +59,7 @@ class FindTopLevelDeclConsumer : public clang::ASTConsumer {
TEST(runToolOnCode, FindsNoTopLevelDeclOnEmptyCode) {
bool FoundTopLevelDecl = false;
EXPECT_TRUE(
- runToolOnCode(new TestAction(llvm::make_unique<FindTopLevelDeclConsumer>(
+ runToolOnCode(llvm::make_unique<TestAction>(llvm::make_unique<FindTopLevelDeclConsumer>(
&FoundTopLevelDecl)),
""));
EXPECT_FALSE(FoundTopLevelDecl);
@@ -100,14 +99,14 @@ bool FindClassDeclX(ASTUnit *AST) {
TEST(runToolOnCode, FindsClassDecl) {
bool FoundClassDeclX = false;
EXPECT_TRUE(
- runToolOnCode(new TestAction(llvm::make_unique<FindClassDeclXConsumer>(
+ runToolOnCode(llvm::make_unique<TestAction>(llvm::make_unique<FindClassDeclXConsumer>(
&FoundClassDeclX)),
"class X;"));
EXPECT_TRUE(FoundClassDeclX);
FoundClassDeclX = false;
EXPECT_TRUE(
- runToolOnCode(new TestAction(llvm::make_unique<FindClassDeclXConsumer>(
+ runToolOnCode(llvm::make_unique<TestAction>(llvm::make_unique<FindClassDeclXConsumer>(
&FoundClassDeclX)),
"class Y;"));
EXPECT_FALSE(FoundClassDeclX);
@@ -152,7 +151,7 @@ TEST(ToolInvocation, TestMapVirtualFile) {
Args.push_back("-Idef");
Args.push_back("-fsyntax-only");
Args.push_back("test.cpp");
- clang::tooling::ToolInvocation Invocation(Args, new SyntaxOnlyAction,
+ clang::tooling::ToolInvocation Invocation(Args, llvm::make_unique<SyntaxOnlyAction>(),
Files.get());
Invocation.mapVirtualFile("test.cpp", "#include <abc>\n");
Invocation.mapVirtualFile("def/abc", "\n");
@@ -171,7 +170,7 @@ TEST(ToolInvocation, TestVirtualModulesCompilation) {
Args.push_back("-Idef");
Args.push_back("-fsyntax-only");
Args.push_back("test.cpp");
- clang::tooling::ToolInvocation Invocation(Args, new SyntaxOnlyAction,
+ clang::tooling::ToolInvocation Invocation(Args, llvm::make_unique<SyntaxOnlyAction>(),
Files.get());
Invocation.mapVirtualFile("test.cpp", "#include <abc>\n");
Invocation.mapVirtualFile("def/abc", "\n");
@@ -239,9 +238,9 @@ struct SkipBodyAction : public clang::ASTFrontendAction {
};
TEST(runToolOnCode, TestSkipFunctionBody) {
- EXPECT_TRUE(runToolOnCode(new SkipBodyAction,
+ EXPECT_TRUE(runToolOnCode(llvm::make_unique<SkipBodyAction>(),
"int skipMe() { an_error_here }"));
- EXPECT_FALSE(runToolOnCode(new SkipBodyAction,
+ EXPECT_FALSE(runToolOnCode(llvm::make_unique<SkipBodyAction>(),
"int skipMeNot() { an_error_here }"));
}
@@ -255,7 +254,7 @@ TEST(runToolOnCodeWithArgs, TestNoDepFile) {
Args.push_back(DepFilePath.str());
Args.push_back("-MF");
Args.push_back(DepFilePath.str());
- EXPECT_TRUE(runToolOnCodeWithArgs(new SkipBodyAction, "", Args));
+ EXPECT_TRUE(runToolOnCodeWithArgs(llvm::make_unique<SkipBodyAction>(), "", Args));
EXPECT_FALSE(llvm::sys::fs::exists(DepFilePath.str()));
EXPECT_FALSE(llvm::sys::fs::remove(DepFilePath.str()));
}
More information about the cfe-commits
mailing list