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.

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 <sstream>
 #include <stdlib.h>
 
-class cmExecutionStatus;
-
 cmCTestHandlerCommand::cmCTestHandlerCommand()
 {
   const size_t INIT_SIZE = 100;
@@ -86,7 +85,7 @@ private:
 }
 
 bool cmCTestHandlerCommand::InitialPass(std::vector<std::string> 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<std::string> 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<std::string> 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<std::string> 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<std::string> 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<cmListFileArgument>& args,
                                   cmExecutionStatus& status)
 {
@@ -19,15 +25,7 @@ bool cmCommand::InvokeInitialPass(const std::vector<cmListFileArgument>& 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<cmCommand> 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<std::string> 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 <cmConfigure.h> // IWYU pragma: keep
+#include <string>
+
+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<cmCommand> 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();