From: David Blaikie Date: Fri, 29 Aug 2014 07:25:23 +0000 (+0000) Subject: unique_ptrify Driver Action handling X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f11dfe02ad387ab7421599c1a008a36977866bcc;p=clang unique_ptrify Driver Action handling It hits a limit when we reach ActionList, which is used for dynamic conditional ownership, so we lose type safety there. This did expose at least one caller "lying" about ownership (passing ownership to an Action, then updating the Action to specify that it doesn't actually own the thing that was passed) - changing this to unique_ptr just makes that oddity more obvious. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@216713 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Driver/Action.h b/include/clang/Driver/Action.h index b8de02d548..2739d4934d 100644 --- a/include/clang/Driver/Action.h +++ b/include/clang/Driver/Action.h @@ -72,8 +72,12 @@ private: protected: Action(ActionClass _Kind, types::ID _Type) : Kind(_Kind), Type(_Type), OwnsInputs(true) {} - Action(ActionClass _Kind, Action *Input, types::ID _Type) - : Kind(_Kind), Type(_Type), Inputs(&Input, &Input + 1), OwnsInputs(true) {} + Action(ActionClass _Kind, std::unique_ptr Input, types::ID _Type) + : Kind(_Kind), Type(_Type), Inputs(1, Input.release()), OwnsInputs(true) { + } + Action(ActionClass _Kind, std::unique_ptr Input) + : Kind(_Kind), Type(Input->getType()), Inputs(1, Input.release()), + OwnsInputs(true) {} Action(ActionClass _Kind, const ActionList &_Inputs, types::ID _Type) : Kind(_Kind), Type(_Type), Inputs(_Inputs), OwnsInputs(true) {} public: @@ -119,7 +123,7 @@ class BindArchAction : public Action { const char *ArchName; public: - BindArchAction(Action *Input, const char *_ArchName); + BindArchAction(std::unique_ptr Input, const char *_ArchName); const char *getArchName() const { return ArchName; } @@ -131,7 +135,7 @@ public: class JobAction : public Action { virtual void anchor(); protected: - JobAction(ActionClass Kind, Action *Input, types::ID Type); + JobAction(ActionClass Kind, std::unique_ptr Input, types::ID Type); JobAction(ActionClass Kind, const ActionList &Inputs, types::ID Type); public: @@ -144,7 +148,7 @@ public: class PreprocessJobAction : public JobAction { void anchor() override; public: - PreprocessJobAction(Action *Input, types::ID OutputType); + PreprocessJobAction(std::unique_ptr Input, types::ID OutputType); static bool classof(const Action *A) { return A->getKind() == PreprocessJobClass; @@ -154,7 +158,7 @@ public: class PrecompileJobAction : public JobAction { void anchor() override; public: - PrecompileJobAction(Action *Input, types::ID OutputType); + PrecompileJobAction(std::unique_ptr Input, types::ID OutputType); static bool classof(const Action *A) { return A->getKind() == PrecompileJobClass; @@ -164,7 +168,7 @@ public: class AnalyzeJobAction : public JobAction { void anchor() override; public: - AnalyzeJobAction(Action *Input, types::ID OutputType); + AnalyzeJobAction(std::unique_ptr Input, types::ID OutputType); static bool classof(const Action *A) { return A->getKind() == AnalyzeJobClass; @@ -174,7 +178,7 @@ public: class MigrateJobAction : public JobAction { void anchor() override; public: - MigrateJobAction(Action *Input, types::ID OutputType); + MigrateJobAction(std::unique_ptr Input, types::ID OutputType); static bool classof(const Action *A) { return A->getKind() == MigrateJobClass; @@ -184,7 +188,7 @@ public: class CompileJobAction : public JobAction { void anchor() override; public: - CompileJobAction(Action *Input, types::ID OutputType); + CompileJobAction(std::unique_ptr Input, types::ID OutputType); static bool classof(const Action *A) { return A->getKind() == CompileJobClass; @@ -194,7 +198,7 @@ public: class AssembleJobAction : public JobAction { void anchor() override; public: - AssembleJobAction(Action *Input, types::ID OutputType); + AssembleJobAction(std::unique_ptr Input, types::ID OutputType); static bool classof(const Action *A) { return A->getKind() == AssembleJobClass; @@ -234,7 +238,8 @@ public: class VerifyJobAction : public JobAction { void anchor() override; public: - VerifyJobAction(ActionClass Kind, Action *Input, types::ID Type); + VerifyJobAction(ActionClass Kind, std::unique_ptr Input, + types::ID Type); VerifyJobAction(ActionClass Kind, ActionList &Inputs, types::ID Type); static bool classof(const Action *A) { return A->getKind() == VerifyDebugInfoJobClass || @@ -245,7 +250,7 @@ public: class VerifyDebugInfoJobAction : public VerifyJobAction { void anchor() override; public: - VerifyDebugInfoJobAction(Action *Input, types::ID Type); + VerifyDebugInfoJobAction(std::unique_ptr Input, types::ID Type); static bool classof(const Action *A) { return A->getKind() == VerifyDebugInfoJobClass; } @@ -254,7 +259,7 @@ public: class VerifyPCHJobAction : public VerifyJobAction { void anchor() override; public: - VerifyPCHJobAction(Action *Input, types::ID Type); + VerifyPCHJobAction(std::unique_ptr Input, types::ID Type); static bool classof(const Action *A) { return A->getKind() == VerifyPCHJobClass; } diff --git a/include/clang/Driver/Driver.h b/include/clang/Driver/Driver.h index c563bee3b2..5468a3d62a 100644 --- a/include/clang/Driver/Driver.h +++ b/include/clang/Driver/Driver.h @@ -343,8 +343,9 @@ public: /// ConstructAction - Construct the appropriate action to do for /// \p Phase on the \p Input, taking in to account arguments /// like -fsyntax-only or --analyze. - Action *ConstructPhaseAction(const llvm::opt::ArgList &Args, phases::ID Phase, - Action *Input) const; + std::unique_ptr + ConstructPhaseAction(const llvm::opt::ArgList &Args, phases::ID Phase, + std::unique_ptr Input) const; /// BuildJobsForAction - Construct the jobs to perform for the /// action \p A. diff --git a/lib/Driver/Action.cpp b/lib/Driver/Action.cpp index 86a48fd8b7..d4f339d376 100644 --- a/lib/Driver/Action.cpp +++ b/lib/Driver/Action.cpp @@ -48,15 +48,15 @@ InputAction::InputAction(const Arg &_Input, types::ID _Type) void BindArchAction::anchor() {} -BindArchAction::BindArchAction(Action *Input, const char *_ArchName) - : Action(BindArchClass, Input, Input->getType()), ArchName(_ArchName) { -} +BindArchAction::BindArchAction(std::unique_ptr Input, + const char *_ArchName) + : Action(BindArchClass, std::move(Input)), ArchName(_ArchName) {} void JobAction::anchor() {} -JobAction::JobAction(ActionClass Kind, Action *Input, types::ID Type) - : Action(Kind, Input, Type) { -} +JobAction::JobAction(ActionClass Kind, std::unique_ptr Input, + types::ID Type) + : Action(Kind, std::move(Input), Type) {} JobAction::JobAction(ActionClass Kind, const ActionList &Inputs, types::ID Type) : Action(Kind, Inputs, Type) { @@ -64,39 +64,39 @@ JobAction::JobAction(ActionClass Kind, const ActionList &Inputs, types::ID Type) void PreprocessJobAction::anchor() {} -PreprocessJobAction::PreprocessJobAction(Action *Input, types::ID OutputType) - : JobAction(PreprocessJobClass, Input, OutputType) { -} +PreprocessJobAction::PreprocessJobAction(std::unique_ptr Input, + types::ID OutputType) + : JobAction(PreprocessJobClass, std::move(Input), OutputType) {} void PrecompileJobAction::anchor() {} -PrecompileJobAction::PrecompileJobAction(Action *Input, types::ID OutputType) - : JobAction(PrecompileJobClass, Input, OutputType) { -} +PrecompileJobAction::PrecompileJobAction(std::unique_ptr Input, + types::ID OutputType) + : JobAction(PrecompileJobClass, std::move(Input), OutputType) {} void AnalyzeJobAction::anchor() {} -AnalyzeJobAction::AnalyzeJobAction(Action *Input, types::ID OutputType) - : JobAction(AnalyzeJobClass, Input, OutputType) { -} +AnalyzeJobAction::AnalyzeJobAction(std::unique_ptr Input, + types::ID OutputType) + : JobAction(AnalyzeJobClass, std::move(Input), OutputType) {} void MigrateJobAction::anchor() {} -MigrateJobAction::MigrateJobAction(Action *Input, types::ID OutputType) - : JobAction(MigrateJobClass, Input, OutputType) { -} +MigrateJobAction::MigrateJobAction(std::unique_ptr Input, + types::ID OutputType) + : JobAction(MigrateJobClass, std::move(Input), OutputType) {} void CompileJobAction::anchor() {} -CompileJobAction::CompileJobAction(Action *Input, types::ID OutputType) - : JobAction(CompileJobClass, Input, OutputType) { -} +CompileJobAction::CompileJobAction(std::unique_ptr Input, + types::ID OutputType) + : JobAction(CompileJobClass, std::move(Input), OutputType) {} void AssembleJobAction::anchor() {} -AssembleJobAction::AssembleJobAction(Action *Input, types::ID OutputType) - : JobAction(AssembleJobClass, Input, OutputType) { -} +AssembleJobAction::AssembleJobAction(std::unique_ptr Input, + types::ID OutputType) + : JobAction(AssembleJobClass, std::move(Input), OutputType) {} void LinkJobAction::anchor() {} @@ -118,9 +118,9 @@ DsymutilJobAction::DsymutilJobAction(ActionList &Inputs, types::ID Type) void VerifyJobAction::anchor() {} -VerifyJobAction::VerifyJobAction(ActionClass Kind, Action *Input, - types::ID Type) - : JobAction(Kind, Input, Type) { +VerifyJobAction::VerifyJobAction(ActionClass Kind, + std::unique_ptr Input, types::ID Type) + : JobAction(Kind, std::move(Input), Type) { assert((Kind == VerifyDebugInfoJobClass || Kind == VerifyPCHJobClass) && "ActionClass is not a valid VerifyJobAction"); } @@ -134,13 +134,12 @@ VerifyJobAction::VerifyJobAction(ActionClass Kind, ActionList &Inputs, void VerifyDebugInfoJobAction::anchor() {} -VerifyDebugInfoJobAction::VerifyDebugInfoJobAction(Action *Input, - types::ID Type) - : VerifyJobAction(VerifyDebugInfoJobClass, Input, Type) { -} +VerifyDebugInfoJobAction::VerifyDebugInfoJobAction( + std::unique_ptr Input, types::ID Type) + : VerifyJobAction(VerifyDebugInfoJobClass, std::move(Input), Type) {} void VerifyPCHJobAction::anchor() {} -VerifyPCHJobAction::VerifyPCHJobAction(Action *Input, types::ID Type) - : VerifyJobAction(VerifyPCHJobClass, Input, Type) { -} +VerifyPCHJobAction::VerifyPCHJobAction(std::unique_ptr Input, + types::ID Type) + : VerifyJobAction(VerifyPCHJobClass, std::move(Input), Type) {} diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp index acacb3e88b..e2e6abceef 100644 --- a/lib/Driver/Driver.cpp +++ b/lib/Driver/Driver.cpp @@ -906,7 +906,8 @@ void Driver::BuildUniversalActions(const ToolChain &TC, ActionList Inputs; for (unsigned i = 0, e = Archs.size(); i != e; ++i) { - Inputs.push_back(new BindArchAction(Act, Archs[i])); + Inputs.push_back( + new BindArchAction(std::unique_ptr(Act), Archs[i])); if (i != 0) Inputs.back()->setOwnsInputs(false); } @@ -937,9 +938,9 @@ void Driver::BuildUniversalActions(const ToolChain &TC, // Verify the debug info output. if (Args.hasArg(options::OPT_verify_debug_info)) { - Action *VerifyInput = Actions.back(); + std::unique_ptr VerifyInput(Actions.back()); Actions.pop_back(); - Actions.push_back(new VerifyDebugInfoJobAction(VerifyInput, + Actions.push_back(new VerifyDebugInfoJobAction(std::move(VerifyInput), types::TY_Nothing)); } } @@ -1249,7 +1250,7 @@ void Driver::BuildActions(const ToolChain &TC, DerivedArgList &Args, continue; // Otherwise construct the appropriate action. - Current.reset(ConstructPhaseAction(Args, Phase, Current.release())); + Current = ConstructPhaseAction(Args, Phase, std::move(Current)); if (Current->getType() == types::TY_Nothing) break; } @@ -1274,8 +1275,9 @@ void Driver::BuildActions(const ToolChain &TC, DerivedArgList &Args, Args.ClaimAllArgs(options::OPT_cl_ignored_Group); } -Action *Driver::ConstructPhaseAction(const ArgList &Args, phases::ID Phase, - Action *Input) const { +std::unique_ptr +Driver::ConstructPhaseAction(const ArgList &Args, phases::ID Phase, + std::unique_ptr Input) const { llvm::PrettyStackTraceString CrashInfo("Constructing phase actions"); // Build the appropriate action. switch (Phase) { @@ -1294,7 +1296,7 @@ Action *Driver::ConstructPhaseAction(const ArgList &Args, phases::ID Phase, assert(OutputTy != types::TY_INVALID && "Cannot preprocess this input type!"); } - return new PreprocessJobAction(Input, OutputTy); + return llvm::make_unique(std::move(Input), OutputTy); } case phases::Precompile: { types::ID OutputTy = types::TY_PCH; @@ -1302,39 +1304,49 @@ Action *Driver::ConstructPhaseAction(const ArgList &Args, phases::ID Phase, // Syntax checks should not emit a PCH file OutputTy = types::TY_Nothing; } - return new PrecompileJobAction(Input, OutputTy); + return llvm::make_unique(std::move(Input), OutputTy); } case phases::Compile: { - if (Args.hasArg(options::OPT_fsyntax_only)) { - return new CompileJobAction(Input, types::TY_Nothing); - } else if (Args.hasArg(options::OPT_rewrite_objc)) { - return new CompileJobAction(Input, types::TY_RewrittenObjC); - } else if (Args.hasArg(options::OPT_rewrite_legacy_objc)) { - return new CompileJobAction(Input, types::TY_RewrittenLegacyObjC); - } else if (Args.hasArg(options::OPT__analyze, options::OPT__analyze_auto)) { - return new AnalyzeJobAction(Input, types::TY_Plist); - } else if (Args.hasArg(options::OPT__migrate)) { - return new MigrateJobAction(Input, types::TY_Remap); - } else if (Args.hasArg(options::OPT_emit_ast)) { - return new CompileJobAction(Input, types::TY_AST); - } else if (Args.hasArg(options::OPT_module_file_info)) { - return new CompileJobAction(Input, types::TY_ModuleFile); - } else if (Args.hasArg(options::OPT_verify_pch)) { - return new VerifyPCHJobAction(Input, types::TY_Nothing); - } else if (IsUsingLTO(Args)) { + if (Args.hasArg(options::OPT_fsyntax_only)) + return llvm::make_unique(std::move(Input), + types::TY_Nothing); + if (Args.hasArg(options::OPT_rewrite_objc)) + return llvm::make_unique(std::move(Input), + types::TY_RewrittenObjC); + if (Args.hasArg(options::OPT_rewrite_legacy_objc)) + return llvm::make_unique(std::move(Input), + types::TY_RewrittenLegacyObjC); + if (Args.hasArg(options::OPT__analyze, options::OPT__analyze_auto)) + return llvm::make_unique(std::move(Input), + types::TY_Plist); + if (Args.hasArg(options::OPT__migrate)) + return llvm::make_unique(std::move(Input), + types::TY_Remap); + if (Args.hasArg(options::OPT_emit_ast)) + return llvm::make_unique(std::move(Input), + types::TY_AST); + if (Args.hasArg(options::OPT_module_file_info)) + return llvm::make_unique(std::move(Input), + types::TY_ModuleFile); + if (Args.hasArg(options::OPT_verify_pch)) + return llvm::make_unique(std::move(Input), + types::TY_Nothing); + if (IsUsingLTO(Args)) { types::ID Output = Args.hasArg(options::OPT_S) ? types::TY_LTO_IR : types::TY_LTO_BC; - return new CompileJobAction(Input, Output); - } else if (Args.hasArg(options::OPT_emit_llvm)) { + return llvm::make_unique(std::move(Input), Output); + } + if (Args.hasArg(options::OPT_emit_llvm)) { types::ID Output = Args.hasArg(options::OPT_S) ? types::TY_LLVM_IR : types::TY_LLVM_BC; - return new CompileJobAction(Input, Output); - } else { - return new CompileJobAction(Input, types::TY_PP_Asm); + return llvm::make_unique(std::move(Input), Output); } + return llvm::make_unique(std::move(Input), + types::TY_PP_Asm); } case phases::Assemble: - return new AssembleJobAction(Input, types::TY_Object); + return llvm::make_unique(std::move(Input), + types::TY_Object); } llvm_unreachable("invalid phase in ConstructPhaseAction");