cmState: Hold commands by value

Source/cmCommand.cxx

@@ -2,6 +2,8 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmCommand.h"
 
+#include <utility>
+
 #include "cmExecutionStatus.h"
 #include "cmMakefile.h"
 
@@ -29,3 +31,29 @@ void cmCommand::SetError(const std::string& e)
 {
   this->Status->SetError(e);
 }
+
+cmLegacyCommandWrapper::cmLegacyCommandWrapper(std::unique_ptr<cmCommand> cmd)
+  : Command(std::move(cmd))
+{
+}
+
+cmLegacyCommandWrapper::cmLegacyCommandWrapper(
+  cmLegacyCommandWrapper const& other)
+  : Command(other.Command->Clone())
+{
+}
+
+cmLegacyCommandWrapper& cmLegacyCommandWrapper::operator=(
+  cmLegacyCommandWrapper const& other)
+{
+  this->Command = other.Command->Clone();
+  return *this;
+}
+
+bool cmLegacyCommandWrapper::operator()(
+  std::vector<cmListFileArgument> const& args, cmExecutionStatus& status) const
+{
+  auto cmd = this->Command->Clone();
+  cmd->SetExecutionStatus(&status);
+  return cmd->InvokeInitialPass(args, status);
+}

Source/cmCommand.h

@@ -79,4 +79,22 @@ private:
   cmExecutionStatus* Status = nullptr;
 };
 
+class cmLegacyCommandWrapper
+{
+public:
+  explicit cmLegacyCommandWrapper(std::unique_ptr<cmCommand> cmd);
+
+  cmLegacyCommandWrapper(cmLegacyCommandWrapper const& other);
+  cmLegacyCommandWrapper& operator=(cmLegacyCommandWrapper const& other);
+
+  cmLegacyCommandWrapper(cmLegacyCommandWrapper&&) = default;
+  cmLegacyCommandWrapper& operator=(cmLegacyCommandWrapper&&) = default;
+
+  bool operator()(std::vector<cmListFileArgument> const& args,
+                  cmExecutionStatus& status) const;
+
+private:
+  std::unique_ptr<cmCommand> Command;
+};
+
 #endif

Source/cmConditionEvaluator.cxx

@@ -4,6 +4,7 @@
 
 #include "cmsys/RegularExpression.hxx"
 #include <algorithm>
+#include <functional>
 #include <sstream>
 #include <stdio.h>
 #include <stdlib.h>
@@ -17,7 +18,6 @@
 #include "cmSystemTools.h"
 #include "cmake.h"
 
-class cmCommand;
 class cmTest;
 
 static std::string const keyAND = "AND";
@@ -452,7 +452,7 @@ bool cmConditionEvaluator::HandleLevel1(cmArgumentList& newArgs, std::string&,
       }
       // does a command exist
       if (this->IsKeyword(keyCOMMAND, *arg) && argP1 != newArgs.end()) {
-        cmCommand* command =
+        cmState::Command command =
           this->Makefile.GetState()->GetCommand(argP1->c_str());
         this->HandlePredicate(command != nullptr, reducible, arg, newArgs,
                               argP1, argP2);

Source/cmMakefile.cxx

@@ -16,7 +16,6 @@
 #include <utility>
 
 #include "cmAlgorithms.h"
-#include "cmCommand.h"
 #include "cmCommandArgumentParserHelper.h"
 #include "cmCustomCommand.h"
 #include "cmCustomCommandLines.h"
@@ -388,12 +387,8 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
   }
 
   // Lookup the command prototype.
-  if (cmCommand* proto =
+  if (cmState::Command command =
         this->GetState()->GetCommandByExactName(lff.Name.Lower)) {
-    // Clone the prototype.
-    std::unique_ptr<cmCommand> pcmd(proto->Clone());
-    pcmd->SetExecutionStatus(&status);
-
     // Decide whether to invoke the command.
     if (!cmSystemTools::GetFatalErrorOccured()) {
       // if trace is enabled, print out invoke information
@@ -401,7 +396,7 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
         this->PrintCommandTrace(lff);
       }
       // Try invoking the command.
-      bool invokeSucceeded = pcmd->InvokeInitialPass(lff.Arguments, status);
+      bool invokeSucceeded = command(lff.Arguments, status);
       bool hadNestedError = status.GetNestedError();
       if (!invokeSucceeded || hadNestedError) {
         if (!hadNestedError) {

Source/cmState.cxx

@@ -6,7 +6,6 @@
 #include <algorithm>
 #include <assert.h>
 #include <string.h>
-#include <type_traits>
 #include <utility>
 
 #include "cm_memory.hxx"
@@ -423,9 +422,8 @@ void cmState::AddBuiltinCommand(std::string const& name,
 {
   assert(name == cmSystemTools::LowerCase(name));
   assert(this->BuiltinCommands.find(name) == this->BuiltinCommands.end());
-  this->BuiltinCommands.insert(
-    std::map<std::string, std::unique_ptr<cmCommand>>::value_type(
-      name, std::move(command)));
+  this->BuiltinCommands.emplace(name,
+                                cmLegacyCommandWrapper(std::move(command)));
 }
 
 void cmState::AddDisallowedCommand(std::string const& name,
@@ -450,39 +448,27 @@ void cmState::AddScriptedCommand(std::string const& name,
   std::string sName = cmSystemTools::LowerCase(name);
 
   // if the command already exists, give a new name to the old command.
-  if (cmCommand* oldCmd = this->GetCommand(sName)) {
-    std::string const newName = "_" + sName;
-    auto pos = this->ScriptedCommands.find(newName);
-    if (pos != this->ScriptedCommands.end()) {
-      this->ScriptedCommands.erase(pos);
-    }
-    this->ScriptedCommands.insert(std::make_pair(newName, oldCmd->Clone()));
+  if (Command oldCmd = this->GetCommand(sName)) {
+    this->ScriptedCommands["_" + sName] = oldCmd;
   }
 
-  // if the command already exists, free the old one
-  auto pos = this->ScriptedCommands.find(sName);
-  if (pos != this->ScriptedCommands.end()) {
-    this->ScriptedCommands.erase(pos);
-  }
-  this->ScriptedCommands.insert(
-    std::map<std::string, std::unique_ptr<cmCommand>>::value_type(
-      sName, std::move(command)));
+  this->ScriptedCommands[sName] = cmLegacyCommandWrapper(std::move(command));
 }
 
-cmCommand* cmState::GetCommand(std::string const& name) const
+cmState::Command cmState::GetCommand(std::string const& name) const
 {
   return GetCommandByExactName(cmSystemTools::LowerCase(name));
 }
 
-cmCommand* cmState::GetCommandByExactName(std::string const& name) const
+cmState::Command cmState::GetCommandByExactName(std::string const& name) const
 {
   auto pos = this->ScriptedCommands.find(name);
   if (pos != this->ScriptedCommands.end()) {
-    return pos->second.get();
+    return pos->second;
   }
   pos = this->BuiltinCommands.find(name);
   if (pos != this->BuiltinCommands.end()) {
-    return pos->second.get();
+    return pos->second;
   }
   return nullptr;
 }
@@ -507,9 +493,7 @@ std::vector<std::string> cmState::GetCommandNames() const
 void cmState::RemoveBuiltinCommand(std::string const& name)
 {
   assert(name == cmSystemTools::LowerCase(name));
-  auto i = this->BuiltinCommands.find(name);
-  assert(i != this->BuiltinCommands.end());
-  this->BuiltinCommands.erase(i);
+  this->BuiltinCommands.erase(name);
 }
 
 void cmState::RemoveUserDefinedCommands()

Source/cmState.h

@@ -5,6 +5,7 @@
 
 #include "cmConfigure.h" // IWYU pragma: keep
 
+#include <functional>
 #include <map>
 #include <memory>
 #include <set>
@@ -27,6 +28,7 @@ class cmGlobVerificationManager;
 class cmPropertyDefinition;
 class cmStateSnapshot;
 class cmMessenger;
+class cmExecutionStatus;
 
 class cmState
 {
@@ -141,10 +143,13 @@ public:
   bool GetIsGeneratorMultiConfig() const;
   void SetIsGeneratorMultiConfig(bool b);
 
+  using Command = std::function<bool(std::vector<cmListFileArgument> const&,
+                                     cmExecutionStatus&)>;
+
   // Returns a command from its name, case insensitive, or nullptr
-  cmCommand* GetCommand(std::string const& name) const;
+  Command GetCommand(std::string const& name) const;
   // Returns a command from its name, or nullptr
-  cmCommand* GetCommandByExactName(std::string const& name) const;
+  Command GetCommandByExactName(std::string const& name) const;
 
   void AddBuiltinCommand(std::string const& name,
                          std::unique_ptr<cmCommand> command);
@@ -212,8 +217,8 @@ private:
 
   std::map<cmProperty::ScopeType, cmPropertyDefinitionMap> PropertyDefinitions;
   std::vector<std::string> EnabledLanguages;
-  std::map<std::string, std::unique_ptr<cmCommand>> BuiltinCommands;
-  std::map<std::string, std::unique_ptr<cmCommand>> ScriptedCommands;
+  std::map<std::string, Command> BuiltinCommands;
+  std::map<std::string, Command> ScriptedCommands;
   cmPropertyMap GlobalProperties;
   std::unique_ptr<cmCacheManager> CacheManager;
   std::unique_ptr<cmGlobVerificationManager> GlobVerificationManager;