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

@@ -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();
+      std::string const& err = status.GetError();
-      if (err && !cmSystemTools::FindLastString(err, "unknown error.")) {
+      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);
       }
     }

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);
 }
 

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);
 }

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
@@ -65,11 +67,6 @@ public:
    */
   virtual std::unique_ptr<cmCommand> Clone() = 0;
 
-  /**
-   * Return the last error string.
-   */
-  const char* GetError();
-
   /**
    * Set the error message
    */
@@ -79,7 +76,7 @@ protected:
   cmMakefile* Makefile = nullptr;
 
 private:
-  std::string Error;
+  cmExecutionStatus* Status = nullptr;
 };
 
 #endif

Source/cmDisallowedCommand.cxx

@@ -24,8 +24,6 @@ bool cmDisallowedCommand::InitialPass(std::vector<std::string> const& args,
       return true;
   }
 
-  this->Command->SetMakefile(this->GetMakefile());
+  this->Command->SetExecutionStatus(this->GetExecutionStatus());
-  bool const ret = this->Command->InitialPass(args, status);
+  return this->Command->InitialPass(args, status);
-  this->SetError(this->Command->GetError());
-  return ret;
 }

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;

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;
+        this->FileCommand->SetError(
-        e << this->FileCommand->GetError()
+          " Set with CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS variable.");
-          << " Set with CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS "
-             "variable.";
-        this->FileCommand->SetError(e.str());
         return false;
       }
     }

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);

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

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

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

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;

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"

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();