From 1eebc2956321c2e7da00a5d35e207bedb899c804 Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Mon, 26 Dec 2016 10:38:36 +0100 Subject: cmCommand: deprecate functions GetMakefile and SetError Replace the members for the Makefile and the Error with a cmExecutionStatus. Re-implement GetMakefile and SetError based on that. Both functions should be called directly on the cmExecutionStatus that is passed to InitialPass. This will help us make all Commands immutable and remove the need for cloning. --- Source/CTest/cmCTestHandlerCommand.cxx | 17 ++++++++--------- Source/cmCPluginAPI.cxx | 2 +- Source/cmCommand.cxx | 18 ++++++++---------- Source/cmCommand.h | 11 ++++------- Source/cmDisallowedCommand.cxx | 6 ++---- Source/cmExecutionStatus.h | 19 +++++++++++++++++++ Source/cmFileCopier.cxx | 7 ++----- Source/cmForEachCommand.cxx | 2 +- Source/cmFunctionCommand.cxx | 2 +- Source/cmIfCommand.cxx | 2 +- Source/cmMacroCommand.cxx | 2 +- Source/cmMakefile.cxx | 6 +++--- Source/cmVariableWatchCommand.cxx | 2 +- Source/cmWhileCommand.cxx | 2 +- 14 files changed, 53 insertions(+), 45 deletions(-) diff --git a/Source/CTest/cmCTestHandlerCommand.cxx b/Source/CTest/cmCTestHandlerCommand.cxx index adf9553..2b73d40 100644 --- a/Source/CTest/cmCTestHandlerCommand.cxx +++ b/Source/CTest/cmCTestHandlerCommand.cxx @@ -4,6 +4,7 @@ #include "cmCTest.h" #include "cmCTestGenericHandler.h" +#include "cmExecutionStatus.h" #include "cmMakefile.h" #include "cmMessageType.h" #include "cmSystemTools.h" @@ -13,8 +14,6 @@ #include #include -class cmExecutionStatus; - cmCTestHandlerCommand::cmCTestHandlerCommand() { const size_t INIT_SIZE = 100; @@ -86,7 +85,7 @@ private: } bool cmCTestHandlerCommand::InitialPass(std::vector const& args, - cmExecutionStatus& /*unused*/) + cmExecutionStatus& status) { // save error state and restore it if needed SaveRestoreErrorState errorState; @@ -126,7 +125,7 @@ bool cmCTestHandlerCommand::InitialPass(std::vector const& args, if (capureCMakeError) { this->Makefile->AddDefinition(this->Values[ct_CAPTURE_CMAKE_ERROR], "-1"); - std::string const err = this->GetName() + " " + this->GetError(); + std::string const err = this->GetName() + " " + status.GetError(); if (!cmSystemTools::FindLastString(err.c_str(), "unknown error.")) { cmCTestLog(this->CTest, ERROR_MESSAGE, err << " error from command\n"); } @@ -195,8 +194,8 @@ bool cmCTestHandlerCommand::InitialPass(std::vector const& args, if (capureCMakeError) { this->Makefile->AddDefinition(this->Values[ct_CAPTURE_CMAKE_ERROR], "-1"); - const char* err = this->GetError(); - if (err && !cmSystemTools::FindLastString(err, "unknown error.")) { + std::string const& err = status.GetError(); + if (!cmSystemTools::FindLastString(err.c_str(), "unknown error.")) { cmCTestLog(this->CTest, ERROR_MESSAGE, err << " error from command\n"); } return true; @@ -220,7 +219,7 @@ bool cmCTestHandlerCommand::InitialPass(std::vector const& args, this->Makefile->AddDefinition(this->Values[ct_CAPTURE_CMAKE_ERROR], "-1"); cmCTestLog(this->CTest, ERROR_MESSAGE, - this->GetName() << " " << this->GetError() << "\n"); + this->GetName() << " " << status.GetError() << "\n"); // return success because failure is recorded in CAPTURE_CMAKE_ERROR return true; } @@ -240,10 +239,10 @@ bool cmCTestHandlerCommand::InitialPass(std::vector const& args, const char* returnString = "0"; if (cmSystemTools::GetErrorOccuredFlag()) { returnString = "-1"; - const char* err = this->GetError(); + std::string const& err = status.GetError(); // print out the error if it is not "unknown error" which means // there was no message - if (err && !cmSystemTools::FindLastString(err, "unknown error.")) { + if (!cmSystemTools::FindLastString(err.c_str(), "unknown error.")) { cmCTestLog(this->CTest, ERROR_MESSAGE, err); } } diff --git a/Source/cmCPluginAPI.cxx b/Source/cmCPluginAPI.cxx index 255a8e6..8c2d987 100644 --- a/Source/cmCPluginAPI.cxx +++ b/Source/cmCPluginAPI.cxx @@ -421,7 +421,7 @@ int CCONV cmExecuteCommand(void* arg, const char* name, int numArgs, // Assume all arguments are quoted. lff.Arguments.emplace_back(args[i], cmListFileArgument::Quoted, 0); } - cmExecutionStatus status; + cmExecutionStatus status(*mf); return mf->ExecuteCommand(lff, status); } diff --git a/Source/cmCommand.cxx b/Source/cmCommand.cxx index d349c91..99bdd1e 100644 --- a/Source/cmCommand.cxx +++ b/Source/cmCommand.cxx @@ -2,11 +2,17 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmCommand.h" +#include "cmExecutionStatus.h" #include "cmMakefile.h" -class cmExecutionStatus; struct cmListFileArgument; +void cmCommand::SetExecutionStatus(cmExecutionStatus* status) +{ + this->Status = status; + this->Makefile = &status->GetMakefile(); +} + bool cmCommand::InvokeInitialPass(const std::vector& args, cmExecutionStatus& status) { @@ -19,15 +25,7 @@ bool cmCommand::InvokeInitialPass(const std::vector& args, return this->InitialPass(expandedArguments, status); } -const char* cmCommand::GetError() -{ - if (this->Error.empty()) { - return "unknown error."; - } - return this->Error.c_str(); -} - void cmCommand::SetError(const std::string& e) { - this->Error = e; + this->Status->SetError(e); } diff --git a/Source/cmCommand.h b/Source/cmCommand.h index b210f27..6d3a5fa 100644 --- a/Source/cmCommand.h +++ b/Source/cmCommand.h @@ -42,9 +42,11 @@ public: /** * Specify the makefile. */ - void SetMakefile(cmMakefile* m) { this->Makefile = m; } cmMakefile* GetMakefile() { return this->Makefile; } + void SetExecutionStatus(cmExecutionStatus* s); + cmExecutionStatus* GetExecutionStatus() { return this->Status; }; + /** * This is called by the cmMakefile when the command is first * encountered in the CMakeLists.txt file. It expands the command's @@ -66,11 +68,6 @@ public: virtual std::unique_ptr Clone() = 0; /** - * Return the last error string. - */ - const char* GetError(); - - /** * Set the error message */ void SetError(const std::string& e); @@ -79,7 +76,7 @@ protected: cmMakefile* Makefile = nullptr; private: - std::string Error; + cmExecutionStatus* Status = nullptr; }; #endif diff --git a/Source/cmDisallowedCommand.cxx b/Source/cmDisallowedCommand.cxx index 418d98c..aa1f90b 100644 --- a/Source/cmDisallowedCommand.cxx +++ b/Source/cmDisallowedCommand.cxx @@ -24,8 +24,6 @@ bool cmDisallowedCommand::InitialPass(std::vector const& args, return true; } - this->Command->SetMakefile(this->GetMakefile()); - bool const ret = this->Command->InitialPass(args, status); - this->SetError(this->Command->GetError()); - return ret; + this->Command->SetExecutionStatus(this->GetExecutionStatus()); + return this->Command->InitialPass(args, status); } diff --git a/Source/cmExecutionStatus.h b/Source/cmExecutionStatus.h index 56199dd..bcacc2f 100644 --- a/Source/cmExecutionStatus.h +++ b/Source/cmExecutionStatus.h @@ -3,6 +3,11 @@ #ifndef cmExecutionStatus_h #define cmExecutionStatus_h +#include // IWYU pragma: keep +#include + +class cmMakefile; + /** \class cmExecutionStatus * \brief Superclass for all command status classes * @@ -11,14 +16,26 @@ class cmExecutionStatus { public: + cmExecutionStatus(cmMakefile& makefile) + : Makefile(makefile) + , Error("unknown error.") + { + } + void Clear() { + this->Error = "unknown error."; this->ReturnInvoked = false; this->BreakInvoked = false; this->ContinueInvoked = false; this->NestedError = false; } + cmMakefile& GetMakefile() { return this->Makefile; } + + void SetError(std::string const& e) { this->Error = e; } + std::string const& GetError() const { return this->Error; } + void SetReturnInvoked() { this->ReturnInvoked = true; } bool GetReturnInvoked() const { return this->ReturnInvoked; } @@ -32,6 +49,8 @@ public: bool GetNestedError() const { return this->NestedError; } private: + cmMakefile& Makefile; + std::string Error; bool ReturnInvoked = false; bool BreakInvoked = false; bool ContinueInvoked = false; diff --git a/Source/cmFileCopier.cxx b/Source/cmFileCopier.cxx index 49e8cd5..4f1a158 100644 --- a/Source/cmFileCopier.cxx +++ b/Source/cmFileCopier.cxx @@ -174,11 +174,8 @@ bool cmFileCopier::GetDefaultDirectoryPermissions(mode_t** mode) cmSystemTools::ExpandListArgument(default_dir_install_permissions, items); for (const auto& arg : items) { if (!this->CheckPermissions(arg, **mode)) { - std::ostringstream e; - e << this->FileCommand->GetError() - << " Set with CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS " - "variable."; - this->FileCommand->SetError(e.str()); + this->FileCommand->SetError( + " Set with CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS variable."); return false; } } diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx index a30ebe1..e3918b5 100644 --- a/Source/cmForEachCommand.cxx +++ b/Source/cmForEachCommand.cxx @@ -55,7 +55,7 @@ bool cmForEachFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff, // set the variable to the loop value mf.AddDefinition(this->Args[0], arg.c_str()); // Invoke all the functions that were collected in the block. - cmExecutionStatus status; + cmExecutionStatus status(mf); for (cmListFileFunction const& func : this->Functions) { status.Clear(); mf.ExecuteCommand(func, status); diff --git a/Source/cmFunctionCommand.cxx b/Source/cmFunctionCommand.cxx index 6d06531..4041d62 100644 --- a/Source/cmFunctionCommand.cxx +++ b/Source/cmFunctionCommand.cxx @@ -103,7 +103,7 @@ bool cmFunctionHelperCommand::InvokeInitialPass( // Invoke all the functions that were collected in the block. // for each function for (cmListFileFunction const& func : this->Functions) { - cmExecutionStatus status; + cmExecutionStatus status(*this->GetMakefile()); if (!this->Makefile->ExecuteCommand(func, status) || status.GetNestedError()) { // The error message should have already included the call stack diff --git a/Source/cmIfCommand.cxx b/Source/cmIfCommand.cxx index 625dd45..4edea17 100644 --- a/Source/cmIfCommand.cxx +++ b/Source/cmIfCommand.cxx @@ -47,7 +47,7 @@ bool cmIfFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff, } // execute the functions for the true parts of the if statement - cmExecutionStatus status; + cmExecutionStatus status(mf); int scopeDepth = 0; for (cmListFileFunction const& func : this->Functions) { // keep track of scope depth diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 6e65c6b..11f4bf8 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -132,7 +132,7 @@ bool cmMacroHelperCommand::InvokeInitialPass( arg.Line = k.Line; newLFF.Arguments.push_back(std::move(arg)); } - cmExecutionStatus status; + cmExecutionStatus status(*this->GetMakefile()); if (!this->Makefile->ExecuteCommand(newLFF, status) || status.GetNestedError()) { // The error message should have already included the call stack diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 1723c5a..cb7e94b 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -392,7 +392,7 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff, this->GetState()->GetCommandByExactName(lff.Name.Lower)) { // Clone the prototype. std::unique_ptr pcmd(proto->Clone()); - pcmd->SetMakefile(this); + pcmd->SetExecutionStatus(&status); // Decide whether to invoke the command. if (!cmSystemTools::GetFatalErrorOccured()) { @@ -407,7 +407,7 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff, if (!hadNestedError) { // The command invocation requested that we report an error. std::string const error = - std::string(lff.Name.Original) + " " + pcmd->GetError(); + std::string(lff.Name.Original) + " " + status.GetError(); this->IssueMessage(MessageType::FATAL_ERROR, error); } result = false; @@ -657,7 +657,7 @@ void cmMakefile::ReadListFile(cmListFile const& listFile, // Run the parsed commands. const size_t numberFunctions = listFile.Functions.size(); for (size_t i = 0; i < numberFunctions; ++i) { - cmExecutionStatus status; + cmExecutionStatus status(*this); this->ExecuteCommand(listFile.Functions[i], status); if (cmSystemTools::GetFatalErrorOccured()) { break; diff --git a/Source/cmVariableWatchCommand.cxx b/Source/cmVariableWatchCommand.cxx index afc0b76..83a774d 100644 --- a/Source/cmVariableWatchCommand.cxx +++ b/Source/cmVariableWatchCommand.cxx @@ -55,7 +55,7 @@ static void cmVariableWatchCommandVariableAccessed(const std::string& variable, newLFF.Arguments.emplace_back(stack, cmListFileArgument::Quoted, 9999); newLFF.Name = data->Command; newLFF.Line = 9999; - cmExecutionStatus status; + cmExecutionStatus status(*makefile); if (!makefile->ExecuteCommand(newLFF, status)) { std::ostringstream error; error << "Error in cmake code at\nUnknown:0:\n" diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx index a902964..434c298 100644 --- a/Source/cmWhileCommand.cxx +++ b/Source/cmWhileCommand.cxx @@ -82,7 +82,7 @@ bool cmWhileFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff, // Invoke all the functions that were collected in the block. for (cmListFileFunction const& fn : this->Functions) { - cmExecutionStatus status; + cmExecutionStatus status(mf); mf.ExecuteCommand(fn, status); if (status.GetReturnInvoked()) { inStatus.SetReturnInvoked(); -- cgit v0.12