r260048 - [Frontend] Make the memory management of FrontendAction pointers explicit by using unique_ptr.
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 8 17:39:55 PST 2016
FWIW, I tried to do something like this, perhaps with some other
improvements, a few years ago. Not sure if things have changed for the
better since then, but maybe those old patches may provide some
insight/other improvements/options:
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20140915/115122.html
http://reviews.llvm.org/D4313
http://reviews.llvm.org/D4312
On Sun, Feb 7, 2016 at 11:28 AM, Argyrios Kyrtzidis via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: akirtzidis
> Date: Sun Feb 7 13:28:36 2016
> New Revision: 260048
>
> URL: http://llvm.org/viewvc/llvm-project?rev=260048&view=rev
> Log:
> [Frontend] Make the memory management of FrontendAction pointers explicit
> by using unique_ptr.
>
> Modified:
> cfe/trunk/include/clang/ARCMigrate/ARCMTActions.h
> cfe/trunk/include/clang/Frontend/FrontendAction.h
> cfe/trunk/include/clang/Frontend/FrontendActions.h
> cfe/trunk/include/clang/Rewrite/Frontend/FrontendActions.h
> cfe/trunk/lib/ARCMigrate/ARCMTActions.cpp
> cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
> cfe/trunk/lib/Frontend/ASTMerge.cpp
> cfe/trunk/lib/Frontend/FrontendAction.cpp
> cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
>
> Modified: cfe/trunk/include/clang/ARCMigrate/ARCMTActions.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ARCMigrate/ARCMTActions.h?rev=260048&r1=260047&r2=260048&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/ARCMigrate/ARCMTActions.h (original)
> +++ cfe/trunk/include/clang/ARCMigrate/ARCMTActions.h Sun Feb 7 13:28:36
> 2016
> @@ -22,7 +22,7 @@ protected:
> bool BeginInvocation(CompilerInstance &CI) override;
>
> public:
> - CheckAction(FrontendAction *WrappedAction);
> + CheckAction(std::unique_ptr<FrontendAction> WrappedAction);
> };
>
> class ModifyAction : public WrapperFrontendAction {
> @@ -30,7 +30,7 @@ protected:
> bool BeginInvocation(CompilerInstance &CI) override;
>
> public:
> - ModifyAction(FrontendAction *WrappedAction);
> + ModifyAction(std::unique_ptr<FrontendAction> WrappedAction);
> };
>
> class MigrateSourceAction : public ASTFrontendAction {
> @@ -49,7 +49,8 @@ protected:
> bool BeginInvocation(CompilerInstance &CI) override;
>
> public:
> - MigrateAction(FrontendAction *WrappedAction, StringRef migrateDir,
> + MigrateAction(std::unique_ptr<FrontendAction> WrappedAction,
> + StringRef migrateDir,
> StringRef plistOut,
> bool emitPremigrationARCErrors);
> };
> @@ -61,8 +62,8 @@ class ObjCMigrateAction : public Wrapper
> FileRemapper Remapper;
> CompilerInstance *CompInst;
> public:
> - ObjCMigrateAction(FrontendAction *WrappedAction, StringRef migrateDir,
> - unsigned migrateAction);
> + ObjCMigrateAction(std::unique_ptr<FrontendAction> WrappedAction,
> + StringRef migrateDir, unsigned migrateAction);
>
> protected:
> std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
>
> Modified: cfe/trunk/include/clang/Frontend/FrontendAction.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendAction.h?rev=260048&r1=260047&r2=260048&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Frontend/FrontendAction.h (original)
> +++ cfe/trunk/include/clang/Frontend/FrontendAction.h Sun Feb 7 13:28:36
> 2016
> @@ -283,7 +283,7 @@ protected:
> public:
> /// Construct a WrapperFrontendAction from an existing action, taking
> /// ownership of it.
> - WrapperFrontendAction(FrontendAction *WrappedAction);
> + WrapperFrontendAction(std::unique_ptr<FrontendAction> WrappedAction);
>
> bool usesPreprocessorOnly() const override;
> TranslationUnitKind getTranslationUnitKind() override;
>
> Modified: cfe/trunk/include/clang/Frontend/FrontendActions.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendActions.h?rev=260048&r1=260047&r2=260048&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Frontend/FrontendActions.h (original)
> +++ cfe/trunk/include/clang/Frontend/FrontendActions.h Sun Feb 7 13:28:36
> 2016
> @@ -168,7 +168,7 @@ public:
> */
> class ASTMergeAction : public FrontendAction {
> /// \brief The action that the merge action adapts.
> - FrontendAction *AdaptedAction;
> + std::unique_ptr<FrontendAction> AdaptedAction;
>
> /// \brief The set of AST files to merge.
> std::vector<std::string> ASTFiles;
> @@ -184,7 +184,8 @@ protected:
> void EndSourceFileAction() override;
>
> public:
> - ASTMergeAction(FrontendAction *AdaptedAction, ArrayRef<std::string>
> ASTFiles);
> + ASTMergeAction(std::unique_ptr<FrontendAction> AdaptedAction,
> + ArrayRef<std::string> ASTFiles);
> ~ASTMergeAction() override;
>
> bool usesPreprocessorOnly() const override;
>
> Modified: cfe/trunk/include/clang/Rewrite/Frontend/FrontendActions.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Rewrite/Frontend/FrontendActions.h?rev=260048&r1=260047&r2=260048&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Rewrite/Frontend/FrontendActions.h (original)
> +++ cfe/trunk/include/clang/Rewrite/Frontend/FrontendActions.h Sun Feb 7
> 13:28:36 2016
> @@ -50,8 +50,8 @@ public:
> /// frontend action.
> class FixItRecompile : public WrapperFrontendAction {
> public:
> - FixItRecompile(FrontendAction *WrappedAction)
> - : WrapperFrontendAction(WrappedAction) {}
> + FixItRecompile(std::unique_ptr<FrontendAction> WrappedAction)
> + : WrapperFrontendAction(std::move(WrappedAction)) {}
>
> protected:
> bool BeginInvocation(CompilerInstance &CI) override;
>
> Modified: cfe/trunk/lib/ARCMigrate/ARCMTActions.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/ARCMTActions.cpp?rev=260048&r1=260047&r2=260048&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/ARCMigrate/ARCMTActions.cpp (original)
> +++ cfe/trunk/lib/ARCMigrate/ARCMTActions.cpp Sun Feb 7 13:28:36 2016
> @@ -25,8 +25,8 @@ bool CheckAction::BeginInvocation(Compil
> return true;
> }
>
> -CheckAction::CheckAction(FrontendAction *WrappedAction)
> - : WrapperFrontendAction(WrappedAction) {}
> +CheckAction::CheckAction(std::unique_ptr<FrontendAction> WrappedAction)
> + : WrapperFrontendAction(std::move(WrappedAction)) {}
>
> bool ModifyAction::BeginInvocation(CompilerInstance &CI) {
> return !arcmt::applyTransformations(CI.getInvocation(),
> getCurrentInput(),
> @@ -34,8 +34,8 @@ bool ModifyAction::BeginInvocation(Compi
> CI.getDiagnostics().getClient());
> }
>
> -ModifyAction::ModifyAction(FrontendAction *WrappedAction)
> - : WrapperFrontendAction(WrappedAction) {}
> +ModifyAction::ModifyAction(std::unique_ptr<FrontendAction> WrappedAction)
> + : WrapperFrontendAction(std::move(WrappedAction)) {}
>
> bool MigrateAction::BeginInvocation(CompilerInstance &CI) {
> if (arcmt::migrateWithTemporaryFiles(
> @@ -49,11 +49,11 @@ bool MigrateAction::BeginInvocation(Comp
> return true;
> }
>
> -MigrateAction::MigrateAction(FrontendAction *WrappedAction,
> +MigrateAction::MigrateAction(std::unique_ptr<FrontendAction>
> WrappedAction,
> StringRef migrateDir,
> StringRef plistOut,
> bool emitPremigrationARCErrors)
> - : WrapperFrontendAction(WrappedAction), MigrateDir(migrateDir),
> + : WrapperFrontendAction(std::move(WrappedAction)),
> MigrateDir(migrateDir),
> PlistOut(plistOut),
> EmitPremigrationARCErros(emitPremigrationARCErrors) {
> if (MigrateDir.empty())
> MigrateDir = "."; // user current directory if none is given.
>
> Modified: cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/ObjCMT.cpp?rev=260048&r1=260047&r2=260048&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/ARCMigrate/ObjCMT.cpp (original)
> +++ cfe/trunk/lib/ARCMigrate/ObjCMT.cpp Sun Feb 7 13:28:36 2016
> @@ -179,10 +179,11 @@ protected:
>
> }
>
> -ObjCMigrateAction::ObjCMigrateAction(FrontendAction *WrappedAction,
> +ObjCMigrateAction::ObjCMigrateAction(
> + std::unique_ptr<FrontendAction>
> WrappedAction,
> StringRef migrateDir,
> unsigned migrateAction)
> - : WrapperFrontendAction(WrappedAction), MigrateDir(migrateDir),
> + : WrapperFrontendAction(std::move(WrappedAction)),
> MigrateDir(migrateDir),
> ObjCMigAction(migrateAction),
> CompInst(nullptr) {
> if (MigrateDir.empty())
>
> Modified: cfe/trunk/lib/Frontend/ASTMerge.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTMerge.cpp?rev=260048&r1=260047&r2=260048&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Frontend/ASTMerge.cpp (original)
> +++ cfe/trunk/lib/Frontend/ASTMerge.cpp Sun Feb 7 13:28:36 2016
> @@ -83,14 +83,13 @@ void ASTMergeAction::EndSourceFileAction
> return AdaptedAction->EndSourceFileAction();
> }
>
> -ASTMergeAction::ASTMergeAction(FrontendAction *AdaptedAction,
> +ASTMergeAction::ASTMergeAction(std::unique_ptr<FrontendAction>
> adaptedAction,
> ArrayRef<std::string> ASTFiles)
> - : AdaptedAction(AdaptedAction), ASTFiles(ASTFiles.begin(),
> ASTFiles.end()) {
> +: AdaptedAction(std::move(adaptedAction)), ASTFiles(ASTFiles.begin(),
> ASTFiles.end()) {
> assert(AdaptedAction && "ASTMergeAction needs an action to adapt");
> }
>
> ASTMergeAction::~ASTMergeAction() {
> - delete AdaptedAction;
> }
>
> bool ASTMergeAction::usesPreprocessorOnly() const {
>
> Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=260048&r1=260047&r2=260048&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)
> +++ cfe/trunk/lib/Frontend/FrontendAction.cpp Sun Feb 7 13:28:36 2016
> @@ -587,6 +587,7 @@ bool WrapperFrontendAction::hasCodeCompl
> return WrappedAction->hasCodeCompletionSupport();
> }
>
> -WrapperFrontendAction::WrapperFrontendAction(FrontendAction
> *WrappedAction)
> - : WrappedAction(WrappedAction) {}
> +WrapperFrontendAction::WrapperFrontendAction(
> + std::unique_ptr<FrontendAction> WrappedAction)
> + : WrappedAction(std::move(WrappedAction)) {}
>
>
> Modified: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp?rev=260048&r1=260047&r2=260048&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp (original)
> +++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp Sun Feb 7
> 13:28:36 2016
> @@ -31,33 +31,34 @@
> using namespace clang;
> using namespace llvm::opt;
>
> -static FrontendAction *CreateFrontendBaseAction(CompilerInstance &CI) {
> +static std::unique_ptr<FrontendAction>
> +CreateFrontendBaseAction(CompilerInstance &CI) {
> using namespace clang::frontend;
> StringRef Action("unknown");
> (void)Action;
>
> switch (CI.getFrontendOpts().ProgramAction) {
> - case ASTDeclList: return new ASTDeclListAction();
> - case ASTDump: return new ASTDumpAction();
> - case ASTPrint: return new ASTPrintAction();
> - case ASTView: return new ASTViewAction();
> - case DumpRawTokens: return new DumpRawTokensAction();
> - case DumpTokens: return new DumpTokensAction();
> - case EmitAssembly: return new EmitAssemblyAction();
> - case EmitBC: return new EmitBCAction();
> - case EmitHTML: return new HTMLPrintAction();
> - case EmitLLVM: return new EmitLLVMAction();
> - case EmitLLVMOnly: return new EmitLLVMOnlyAction();
> - case EmitCodeGenOnly: return new EmitCodeGenOnlyAction();
> - case EmitObj: return new EmitObjAction();
> - case FixIt: return new FixItAction();
> - case GenerateModule: return new GenerateModuleAction;
> - case GeneratePCH: return new GeneratePCHAction;
> - case GeneratePTH: return new GeneratePTHAction();
> - case InitOnly: return new InitOnlyAction();
> - case ParseSyntaxOnly: return new SyntaxOnlyAction();
> - case ModuleFileInfo: return new DumpModuleInfoAction();
> - case VerifyPCH: return new VerifyPCHAction();
> + case ASTDeclList: return
> llvm::make_unique<ASTDeclListAction>();
> + case ASTDump: return llvm::make_unique<ASTDumpAction>();
> + case ASTPrint: return llvm::make_unique<ASTPrintAction>();
> + case ASTView: return llvm::make_unique<ASTViewAction>();
> + case DumpRawTokens: return
> llvm::make_unique<DumpRawTokensAction>();
> + case DumpTokens: return
> llvm::make_unique<DumpTokensAction>();
> + case EmitAssembly: return
> llvm::make_unique<EmitAssemblyAction>();
> + case EmitBC: return llvm::make_unique<EmitBCAction>();
> + case EmitHTML: return
> llvm::make_unique<HTMLPrintAction>();
> + case EmitLLVM: return llvm::make_unique<EmitLLVMAction>();
> + case EmitLLVMOnly: return
> llvm::make_unique<EmitLLVMOnlyAction>();
> + case EmitCodeGenOnly: return
> llvm::make_unique<EmitCodeGenOnlyAction>();
> + case EmitObj: return llvm::make_unique<EmitObjAction>();
> + case FixIt: return llvm::make_unique<FixItAction>();
> + case GenerateModule: return
> llvm::make_unique<GenerateModuleAction>();
> + case GeneratePCH: return
> llvm::make_unique<GeneratePCHAction>();
> + case GeneratePTH: return
> llvm::make_unique<GeneratePTHAction>();
> + case InitOnly: return llvm::make_unique<InitOnlyAction>();
> + case ParseSyntaxOnly: return
> llvm::make_unique<SyntaxOnlyAction>();
> + case ModuleFileInfo: return
> llvm::make_unique<DumpModuleInfoAction>();
> + case VerifyPCH: return
> llvm::make_unique<VerifyPCHAction>();
>
> case PluginAction: {
> for (FrontendPluginRegistry::iterator it =
> @@ -67,7 +68,7 @@ static FrontendAction *CreateFrontendBas
> std::unique_ptr<PluginASTAction> P(it->instantiate());
> if (!P->ParseArgs(CI, CI.getFrontendOpts().PluginArgs))
> return nullptr;
> - return P.release();
> + return std::move(P);
> }
> }
>
> @@ -76,32 +77,33 @@ static FrontendAction *CreateFrontendBas
> return nullptr;
> }
>
> - case PrintDeclContext: return new DeclContextPrintAction();
> - case PrintPreamble: return new PrintPreambleAction();
> + case PrintDeclContext: return
> llvm::make_unique<DeclContextPrintAction>();
> + case PrintPreamble: return
> llvm::make_unique<PrintPreambleAction>();
> case PrintPreprocessedInput: {
> if (CI.getPreprocessorOutputOpts().RewriteIncludes)
> - return new RewriteIncludesAction();
> - return new PrintPreprocessedAction();
> + return llvm::make_unique<RewriteIncludesAction>();
> + return llvm::make_unique<PrintPreprocessedAction>();
> }
>
> - case RewriteMacros: return new RewriteMacrosAction();
> - case RewriteTest: return new RewriteTestAction();
> + case RewriteMacros: return
> llvm::make_unique<RewriteMacrosAction>();
> + case RewriteTest: return
> llvm::make_unique<RewriteTestAction>();
> #ifdef CLANG_ENABLE_OBJC_REWRITER
> - case RewriteObjC: return new RewriteObjCAction();
> + case RewriteObjC: return
> llvm::make_unique<RewriteObjCAction>();
> #else
> case RewriteObjC: Action = "RewriteObjC"; break;
> #endif
> #ifdef CLANG_ENABLE_ARCMT
> - case MigrateSource: return new arcmt::MigrateSourceAction();
> + case MigrateSource:
> + return llvm::make_unique<arcmt::MigrateSourceAction>();
> #else
> case MigrateSource: Action = "MigrateSource"; break;
> #endif
> #ifdef CLANG_ENABLE_STATIC_ANALYZER
> - case RunAnalysis: return new ento::AnalysisAction();
> + case RunAnalysis: return
> llvm::make_unique<ento::AnalysisAction>();
> #else
> case RunAnalysis: Action = "RunAnalysis"; break;
> #endif
> - case RunPreprocessorOnly: return new PreprocessOnlyAction();
> + case RunPreprocessorOnly: return
> llvm::make_unique<PreprocessOnlyAction>();
> }
>
> #if !defined(CLANG_ENABLE_ARCMT) ||
> !defined(CLANG_ENABLE_STATIC_ANALYZER) \
> @@ -113,16 +115,17 @@ static FrontendAction *CreateFrontendBas
> #endif
> }
>
> -static FrontendAction *CreateFrontendAction(CompilerInstance &CI) {
> +static std::unique_ptr<FrontendAction>
> +CreateFrontendAction(CompilerInstance &CI) {
> // Create the underlying action.
> - FrontendAction *Act = CreateFrontendBaseAction(CI);
> + std::unique_ptr<FrontendAction> Act = CreateFrontendBaseAction(CI);
> if (!Act)
> return nullptr;
>
> const FrontendOptions &FEOpts = CI.getFrontendOpts();
>
> if (FEOpts.FixAndRecompile) {
> - Act = new FixItRecompile(Act);
> + Act = llvm::make_unique<FixItRecompile>(std::move(Act));
> }
>
> #ifdef CLANG_ENABLE_ARCMT
> @@ -133,13 +136,13 @@ static FrontendAction *CreateFrontendAct
> case FrontendOptions::ARCMT_None:
> break;
> case FrontendOptions::ARCMT_Check:
> - Act = new arcmt::CheckAction(Act);
> + Act = llvm::make_unique<arcmt::CheckAction>(std::move(Act));
> break;
> case FrontendOptions::ARCMT_Modify:
> - Act = new arcmt::ModifyAction(Act);
> + Act = llvm::make_unique<arcmt::ModifyAction>(std::move(Act));
> break;
> case FrontendOptions::ARCMT_Migrate:
> - Act = new arcmt::MigrateAction(Act,
> + Act = llvm::make_unique<arcmt::MigrateAction>(std::move(Act),
> FEOpts.MTMigrateDir,
> FEOpts.ARCMTMigrateReportOut,
> FEOpts.ARCMTMigrateEmitARCErrors);
> @@ -147,8 +150,9 @@ static FrontendAction *CreateFrontendAct
> }
>
> if (FEOpts.ObjCMTAction != FrontendOptions::ObjCMT_None) {
> - Act = new arcmt::ObjCMigrateAction(Act, FEOpts.MTMigrateDir,
> - FEOpts.ObjCMTAction);
> + Act = llvm::make_unique<arcmt::ObjCMigrateAction>(std::move(Act),
> +
> FEOpts.MTMigrateDir,
> +
> FEOpts.ObjCMTAction);
> }
> }
> #endif
> @@ -156,7 +160,8 @@ static FrontendAction *CreateFrontendAct
> // If there are any AST files to merge, create a frontend action
> // adaptor to perform the merge.
> if (!FEOpts.ASTMergeFiles.empty())
> - Act = new ASTMergeAction(Act, FEOpts.ASTMergeFiles);
> + Act = llvm::make_unique<ASTMergeAction>(std::move(Act),
> + FEOpts.ASTMergeFiles);
>
> return Act;
> }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160208/f2e062f1/attachment-0001.html>
More information about the cfe-commits
mailing list