Merge topic 'command-final-action'

732dd344b9 cmCommand: remove FinalPass from interface
fbee46e262 cmVariableWatchCommand: Port away from FinalPass
360d415592 cmLoadCommandCommand: Port away from FinalPass
316e40baec cmInstallProgramsCommand: Port away from FinalPass
7bc88b9165 cmInstallFilesCommand: Port away from FinalPass
6a1a3763ee cmFLTKWrapUICommand: Port away from FinalPass
20169f0b8d cmExportLibraryDependenciesCommand: Port away from FinalPass
a74dad3bd3 cmMakefile: decouple FinalAction from cmCommand
...

Acked-by: Kitware Robot <[email protected]>
Merge-request: !3562

Source/cmCommand.h

@@ -60,19 +60,6 @@ public:
   virtual bool InitialPass(std::vector<std::string> const& args,
                            cmExecutionStatus&) = 0;
 
-  /**
-   * This is called at the end after all the information
-   * specified by the command is accumulated. Most commands do
-   * not implement this method.  At this point, reading and
-   * writing to the cache can be done.
-   */
-  virtual void FinalPass() {}
-
-  /**
-   * Does this command have a final pass?  Query after InitialPass.
-   */
-  virtual bool HasFinalPass() const { return false; }
-
   /**
    * This is a virtual constructor for the command.
    */

Source/cmDisallowedCommand.h

@@ -38,10 +38,6 @@ public:
   bool InitialPass(std::vector<std::string> const& args,
                    cmExecutionStatus& status) override;
 
-  void FinalPass() override { this->Command->FinalPass(); }
-
-  bool HasFinalPass() const override { return this->Command->HasFinalPass(); }
-
 private:
   std::unique_ptr<cmCommand> Command;
   cmPolicies::PolicyID Policy;

Source/cmExportLibraryDependenciesCommand.cxx

@@ -19,57 +19,31 @@
 
 class cmExecutionStatus;
 
-bool cmExportLibraryDependenciesCommand::InitialPass(
-  std::vector<std::string> const& args, cmExecutionStatus&)
-{
-  if (args.empty()) {
-    this->SetError("called with incorrect number of arguments");
-    return false;
-  }
-
-  // store the arguments for the final pass
-  this->Filename = args[0];
-  this->Append = false;
-  if (args.size() > 1) {
-    if (args[1] == "APPEND") {
-      this->Append = true;
-    }
-  }
-  return true;
-}
-
-void cmExportLibraryDependenciesCommand::FinalPass()
-{
-  // export_library_dependencies() shouldn't modify anything
-  // ensure this by calling a const method
-  this->ConstFinalPass();
-}
-
-void cmExportLibraryDependenciesCommand::ConstFinalPass() const
+static void FinalAction(cmMakefile& makefile, std::string const& filename,
+                        bool append)
 {
   // Use copy-if-different if not appending.
   std::unique_ptr<cmsys::ofstream> foutPtr;
-  if (this->Append) {
+  if (append) {
     const auto openmodeApp = std::ios::app;
-    foutPtr =
-      cm::make_unique<cmsys::ofstream>(this->Filename.c_str(), openmodeApp);
+    foutPtr = cm::make_unique<cmsys::ofstream>(filename.c_str(), openmodeApp);
   } else {
     std::unique_ptr<cmGeneratedFileStream> ap(
-      new cmGeneratedFileStream(this->Filename, true));
+      new cmGeneratedFileStream(filename, true));
     ap->SetCopyIfDifferent(true);
     foutPtr = std::move(ap);
   }
   std::ostream& fout = *foutPtr;
 
   if (!fout) {
-    cmSystemTools::Error("Error Writing " + this->Filename);
+    cmSystemTools::Error("Error Writing " + filename);
     cmSystemTools::ReportLastSystemError("");
     return;
   }
 
   // Collect dependency information about all library targets built in
   // the project.
-  cmake* cm = this->Makefile->GetCMakeInstance();
+  cmake* cm = makefile.GetCMakeInstance();
   cmGlobalGenerator* global = cm->GetGlobalGenerator();
   const std::vector<cmMakefile*>& locals = global->GetMakefiles();
   std::map<std::string, std::string> libDepsOld;
@@ -166,3 +140,20 @@ void cmExportLibraryDependenciesCommand::ConstFinalPass() const
   }
   fout << "endif()\n";
 }
+
+bool cmExportLibraryDependenciesCommand::InitialPass(
+  std::vector<std::string> const& args, cmExecutionStatus&)
+{
+  if (args.empty()) {
+    this->SetError("called with incorrect number of arguments");
+    return false;
+  }
+
+  std::string const& filename = args[0];
+  bool const append = args.size() > 1 && args[1] == "APPEND";
+  this->Makefile->AddFinalAction([filename, append](cmMakefile& makefile) {
+    FinalAction(makefile, filename, append);
+  });
+
+  return true;
+}

Source/cmExportLibraryDependenciesCommand.h

@@ -23,14 +23,6 @@ public:
   }
   bool InitialPass(std::vector<std::string> const& args,
                    cmExecutionStatus& status) override;
-
-  void FinalPass() override;
-  bool HasFinalPass() const override { return true; }
-
-private:
-  std::string Filename;
-  bool Append = false;
-  void ConstFinalPass() const;
 };
 
 #endif

Source/cmFLTKWrapUICommand.cxx

@@ -13,6 +13,23 @@
 class cmExecutionStatus;
 class cmTarget;
 
+static void FinalAction(cmMakefile& makefile, std::string const& name)
+{
+  // people should add the srcs to the target themselves, but the old command
+  // didn't support that, so check and see if they added the files in and if
+  // they didn;t then print a warning and add then anyhow
+  cmTarget* target = makefile.FindLocalNonAliasTarget(name);
+  if (!target) {
+    std::string msg =
+      "FLTK_WRAP_UI was called with a target that was never created: ";
+    msg += name;
+    msg += ".  The problem was found while processing the source directory: ";
+    msg += makefile.GetCurrentSourceDirectory();
+    msg += ".  This FLTK_WRAP_UI call will be ignored.";
+    cmSystemTools::Message(msg, "Warning");
+  }
+}
+
 // cmFLTKWrapUICommand
 bool cmFLTKWrapUICommand::InitialPass(std::vector<std::string> const& args,
                                       cmExecutionStatus&)
@@ -27,8 +44,8 @@ bool cmFLTKWrapUICommand::InitialPass(std::vector<std::string> const& args,
   std::string const& fluid_exe =
     this->Makefile->GetRequiredDefinition("FLTK_FLUID_EXECUTABLE");
 
-  // get parameter for the command
-  this->Target = args[0]; // Target that will use the generated files
+  // Target that will use the generated files
+  std::string const& target = args[0];
 
   // get the list of GUI files from which .cxx and .h will be generated
   std::string outputDirectory = this->Makefile->GetCurrentBinaryDirectory();
@@ -41,6 +58,9 @@ bool cmFLTKWrapUICommand::InitialPass(std::vector<std::string> const& args,
     this->Makefile->AddIncludeDirectories(outputDirectories);
   }
 
+  // List of produced files.
+  std::vector<cmSourceFile*> generatedSourcesClasses;
+
   for (std::string const& arg : cmMakeRange(args).advance(1)) {
     cmSourceFile* curr = this->Makefile->GetSource(arg);
     // if we should use the source GUI
@@ -84,40 +104,23 @@ bool cmFLTKWrapUICommand::InitialPass(std::vector<std::string> const& args,
       cmSourceFile* sf = this->Makefile->GetSource(cxxres);
       sf->AddDepend(hname);
       sf->AddDepend(origname);
-      this->GeneratedSourcesClasses.push_back(sf);
+      generatedSourcesClasses.push_back(sf);
     }
   }
 
   // create the variable with the list of sources in it
-  size_t lastHeadersClass = this->GeneratedSourcesClasses.size();
+  size_t lastHeadersClass = generatedSourcesClasses.size();
   std::string sourceListValue;
   for (size_t classNum = 0; classNum < lastHeadersClass; classNum++) {
     if (classNum) {
       sourceListValue += ";";
     }
-    sourceListValue += this->GeneratedSourcesClasses[classNum]->GetFullPath();
+    sourceListValue += generatedSourcesClasses[classNum]->GetFullPath();
   }
-  std::string varName = this->Target;
-  varName += "_FLTK_UI_SRCS";
+  std::string const varName = target + "_FLTK_UI_SRCS";
   this->Makefile->AddDefinition(varName, sourceListValue.c_str());
 
+  this->Makefile->AddFinalAction(
+    [target](cmMakefile& makefile) { FinalAction(makefile, target); });
   return true;
 }
-
-void cmFLTKWrapUICommand::FinalPass()
-{
-  // people should add the srcs to the target themselves, but the old command
-  // didn't support that, so check and see if they added the files in and if
-  // they didn;t then print a warning and add then anyhow
-  cmTarget* target = this->Makefile->FindLocalNonAliasTarget(this->Target);
-  if (!target) {
-    std::string msg =
-      "FLTK_WRAP_UI was called with a target that was never created: ";
-    msg += this->Target;
-    msg += ".  The problem was found while processing the source directory: ";
-    msg += this->Makefile->GetCurrentSourceDirectory();
-    msg += ".  This FLTK_WRAP_UI call will be ignored.";
-    cmSystemTools::Message(msg, "Warning");
-    return;
-  }
-}

Source/cmFLTKWrapUICommand.h

@@ -13,7 +13,6 @@
 #include "cmCommand.h"
 
 class cmExecutionStatus;
-class cmSourceFile;
 
 /** \class cmFLTKWrapUICommand
  * \brief Create .h and .cxx files rules for FLTK user interfaces files
@@ -38,27 +37,6 @@ public:
    */
   bool InitialPass(std::vector<std::string> const& args,
                    cmExecutionStatus& status) override;
-
-  /**
-   * This is called at the end after all the information
-   * specified by the command is accumulated. Most commands do
-   * not implement this method.  At this point, reading and
-   * writing to the cache can be done.
-   */
-  void FinalPass() override;
-  bool HasFinalPass() const override { return true; }
-
-private:
-  /**
-   * List of produced files.
-   */
-  std::vector<cmSourceFile*> GeneratedSourcesClasses;
-
-  /**
-   * List of Fluid files that provide the source
-   * generating .cxx and .h files
-   */
-  std::string Target;
 };
 
 #endif

Source/cmInstallFilesCommand.cxx

@@ -2,7 +2,6 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmInstallFilesCommand.h"
 
-#include "cmAlgorithms.h"
 #include "cmGeneratorExpression.h"
 #include "cmGlobalGenerator.h"
 #include "cmInstallFilesGenerator.h"
@@ -13,7 +12,13 @@
 
 class cmExecutionStatus;
 
-// cmExecutableCommand
+static std::string FindInstallSource(cmMakefile& makefile, const char* name);
+static void CreateInstallGenerator(cmMakefile& makefile,
+                                   std::string const& dest,
+                                   std::vector<std::string> const& files);
+static void FinalAction(cmMakefile& makefile, std::string const& dest,
+                        std::vector<std::string> const& args);
+
 bool cmInstallFilesCommand::InitialPass(std::vector<std::string> const& args,
                                         cmExecutionStatus&)
 {
@@ -25,18 +30,20 @@ bool cmInstallFilesCommand::InitialPass(std::vector<std::string> const& args,
   // Enable the install target.
   this->Makefile->GetGlobalGenerator()->EnableInstallTarget();
 
-  this->Destination = args[0];
+  std::string const& dest = args[0];
 
   if ((args.size() > 1) && (args[1] == "FILES")) {
-    this->IsFilesForm = true;
+    std::vector<std::string> files;
     for (std::string const& arg : cmMakeRange(args).advance(2)) {
       // Find the source location for each file listed.
-      this->Files.push_back(this->FindInstallSource(arg.c_str()));
+      files.push_back(FindInstallSource(*this->Makefile, arg.c_str()));
     }
-    this->CreateInstallGenerator();
+    CreateInstallGenerator(*this->Makefile, dest, files);
   } else {
-    this->IsFilesForm = false;
-    cmAppend(this->FinalArgs, args.begin() + 1, args.end());
+    std::vector<std::string> finalArgs(args.begin() + 1, args.end());
+    this->Makefile->AddFinalAction([dest, finalArgs](cmMakefile& makefile) {
+      FinalAction(makefile, dest, finalArgs);
+    });
   }
 
   this->Makefile->GetGlobalGenerator()->AddInstallComponent(
@@ -45,23 +52,20 @@ bool cmInstallFilesCommand::InitialPass(std::vector<std::string> const& args,
   return true;
 }
 
-void cmInstallFilesCommand::FinalPass()
+static void FinalAction(cmMakefile& makefile, std::string const& dest,
+                        std::vector<std::string> const& args)
 {
-  // No final pass for "FILES" form of arguments.
-  if (this->IsFilesForm) {
-    return;
-  }
-
   std::string testf;
-  std::string const& ext = this->FinalArgs[0];
+  std::string const& ext = args[0];
+  std::vector<std::string> installFiles;
 
   // two different options
-  if (this->FinalArgs.size() > 1) {
+  if (args.size() > 1) {
     // now put the files into the list
-    std::vector<std::string>::iterator s = this->FinalArgs.begin();
+    std::vector<std::string>::const_iterator s = args.begin();
     ++s;
     // for each argument, get the files
-    for (; s != this->FinalArgs.end(); ++s) {
+    for (; s != args.end(); ++s) {
       // replace any variables
       std::string const& temps = *s;
       if (!cmSystemTools::GetFilenamePath(temps).empty()) {
@@ -72,30 +76,31 @@ void cmInstallFilesCommand::FinalPass()
       }
 
       // add to the result
-      this->Files.push_back(this->FindInstallSource(testf.c_str()));
+      installFiles.push_back(FindInstallSource(makefile, testf.c_str()));
     }
   } else // reg exp list
   {
     std::vector<std::string> files;
-    std::string const& regex = this->FinalArgs[0];
-    cmSystemTools::Glob(this->Makefile->GetCurrentSourceDirectory(), regex,
-                        files);
+    std::string const& regex = args[0];
+    cmSystemTools::Glob(makefile.GetCurrentSourceDirectory(), regex, files);
 
     std::vector<std::string>::iterator s = files.begin();
     // for each argument, get the files
     for (; s != files.end(); ++s) {
-      this->Files.push_back(this->FindInstallSource(s->c_str()));
+      installFiles.push_back(FindInstallSource(makefile, s->c_str()));
     }
   }
 
-  this->CreateInstallGenerator();
+  CreateInstallGenerator(makefile, dest, installFiles);
 }
 
-void cmInstallFilesCommand::CreateInstallGenerator() const
+static void CreateInstallGenerator(cmMakefile& makefile,
+                                   std::string const& dest,
+                                   std::vector<std::string> const& files)
 {
   // Construct the destination.  This command always installs under
   // the prefix.  We skip the leading slash given by the user.
-  std::string destination = this->Destination.substr(1);
+  std::string destination = dest.substr(1);
   cmSystemTools::ConvertToUnixSlashes(destination);
   if (destination.empty()) {
     destination = ".";
@@ -106,12 +111,12 @@ void cmInstallFilesCommand::CreateInstallGenerator() const
   const char* no_rename = "";
   bool no_exclude_from_all = false;
   std::string no_component =
-    this->Makefile->GetSafeDefinition("CMAKE_INSTALL_DEFAULT_COMPONENT_NAME");
+    makefile.GetSafeDefinition("CMAKE_INSTALL_DEFAULT_COMPONENT_NAME");
   std::vector<std::string> no_configurations;
   cmInstallGenerator::MessageLevel message =
-    cmInstallGenerator::SelectMessageLevel(this->Makefile);
-  this->Makefile->AddInstallGenerator(new cmInstallFilesGenerator(
-    this->Files, destination.c_str(), false, no_permissions, no_configurations,
+    cmInstallGenerator::SelectMessageLevel(&makefile);
+  makefile.AddInstallGenerator(new cmInstallFilesGenerator(
+    files, destination.c_str(), false, no_permissions, no_configurations,
     no_component.c_str(), message, no_exclude_from_all, no_rename));
 }
 
@@ -121,7 +126,7 @@ void cmInstallFilesCommand::CreateInstallGenerator() const
  * present in the build tree.  If a full path is given, it is just
  * returned.
  */
-std::string cmInstallFilesCommand::FindInstallSource(const char* name) const
+static std::string FindInstallSource(cmMakefile& makefile, const char* name)
 {
   if (cmSystemTools::FileIsFullPath(name) ||
       cmGeneratorExpression::Find(name) == 0) {
@@ -130,10 +135,10 @@ std::string cmInstallFilesCommand::FindInstallSource(const char* name) const
   }
 
   // This is a relative path.
-  std::string tb = this->Makefile->GetCurrentBinaryDirectory();
+  std::string tb = makefile.GetCurrentBinaryDirectory();
   tb += "/";
   tb += name;
-  std::string ts = this->Makefile->GetCurrentSourceDirectory();
+  std::string ts = makefile.GetCurrentSourceDirectory();
   ts += "/";
   ts += name;
 

Source/cmInstallFilesCommand.h

@@ -37,25 +37,6 @@ public:
    */
   bool InitialPass(std::vector<std::string> const& args,
                    cmExecutionStatus& status) override;
-
-  /**
-   * This is called at the end after all the information
-   * specified by the command is accumulated. Most commands do
-   * not implement this method.  At this point, reading and
-   * writing to the cache can be done.
-   */
-  void FinalPass() override;
-  bool HasFinalPass() const override { return !this->IsFilesForm; }
-
-protected:
-  void CreateInstallGenerator() const;
-  std::string FindInstallSource(const char* name) const;
-
-private:
-  std::vector<std::string> FinalArgs;
-  bool IsFilesForm = false;
-  std::string Destination;
-  std::vector<std::string> Files;
 };
 
 #endif

Source/cmInstallProgramsCommand.cxx

@@ -2,7 +2,6 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmInstallProgramsCommand.h"
 
-#include "cmAlgorithms.h"
 #include "cmGeneratorExpression.h"
 #include "cmGlobalGenerator.h"
 #include "cmInstallFilesGenerator.h"
@@ -12,6 +11,10 @@
 
 class cmExecutionStatus;
 
+static void FinalAction(cmMakefile& makefile, std::string const& dest,
+                        std::vector<std::string> const& args);
+static std::string FindInstallSource(cmMakefile& makefile, const char* name);
+
 // cmExecutableCommand
 bool cmInstallProgramsCommand::InitialPass(
   std::vector<std::string> const& args, cmExecutionStatus&)
@@ -24,51 +27,55 @@ bool cmInstallProgramsCommand::InitialPass(
   // Enable the install target.
   this->Makefile->GetGlobalGenerator()->EnableInstallTarget();
 
-  this->Destination = args[0];
-
-  cmAppend(this->FinalArgs, args.begin() + 1, args.end());
-
   this->Makefile->GetGlobalGenerator()->AddInstallComponent(
     this->Makefile->GetSafeDefinition("CMAKE_INSTALL_DEFAULT_COMPONENT_NAME"));
 
+  std::string const& dest = args[0];
+  std::vector<std::string> const finalArgs(args.begin() + 1, args.end());
+  this->Makefile->AddFinalAction([dest, finalArgs](cmMakefile& makefile) {
+    FinalAction(makefile, dest, finalArgs);
+  });
   return true;
 }
 
-void cmInstallProgramsCommand::FinalPass()
+static void FinalAction(cmMakefile& makefile, std::string const& dest,
+                        std::vector<std::string> const& args)
 {
   bool files_mode = false;
-  if (!this->FinalArgs.empty() && this->FinalArgs[0] == "FILES") {
+  if (!args.empty() && args[0] == "FILES") {
     files_mode = true;
   }
 
+  std::vector<std::string> files;
+
   // two different options
-  if (this->FinalArgs.size() > 1 || files_mode) {
+  if (args.size() > 1 || files_mode) {
     // for each argument, get the programs
-    std::vector<std::string>::iterator s = this->FinalArgs.begin();
+    std::vector<std::string>::const_iterator s = args.begin();
     if (files_mode) {
       // Skip the FILES argument in files mode.
       ++s;
     }
-    for (; s != this->FinalArgs.end(); ++s) {
+    for (; s != args.end(); ++s) {
       // add to the result
-      this->Files.push_back(this->FindInstallSource(s->c_str()));
+      files.push_back(FindInstallSource(makefile, s->c_str()));
     }
   } else // reg exp list
   {
     std::vector<std::string> programs;
-    cmSystemTools::Glob(this->Makefile->GetCurrentSourceDirectory(),
-                        this->FinalArgs[0], programs);
+    cmSystemTools::Glob(makefile.GetCurrentSourceDirectory(), args[0],
+                        programs);
 
     std::vector<std::string>::iterator s = programs.begin();
     // for each argument, get the programs
     for (; s != programs.end(); ++s) {
-      this->Files.push_back(this->FindInstallSource(s->c_str()));
+      files.push_back(FindInstallSource(makefile, s->c_str()));
     }
   }
 
   // Construct the destination.  This command always installs under
   // the prefix.  We skip the leading slash given by the user.
-  std::string destination = this->Destination.substr(1);
+  std::string destination = dest.substr(1);
   cmSystemTools::ConvertToUnixSlashes(destination);
   if (destination.empty()) {
     destination = ".";
@@ -79,12 +86,12 @@ void cmInstallProgramsCommand::FinalPass()
   const char* no_rename = "";
   bool no_exclude_from_all = false;
   std::string no_component =
-    this->Makefile->GetSafeDefinition("CMAKE_INSTALL_DEFAULT_COMPONENT_NAME");
+    makefile.GetSafeDefinition("CMAKE_INSTALL_DEFAULT_COMPONENT_NAME");
   std::vector<std::string> no_configurations;
   cmInstallGenerator::MessageLevel message =
-    cmInstallGenerator::SelectMessageLevel(this->Makefile);
-  this->Makefile->AddInstallGenerator(new cmInstallFilesGenerator(
-    this->Files, destination.c_str(), true, no_permissions, no_configurations,
+    cmInstallGenerator::SelectMessageLevel(&makefile);
+  makefile.AddInstallGenerator(new cmInstallFilesGenerator(
+    files, destination.c_str(), true, no_permissions, no_configurations,
     no_component.c_str(), message, no_exclude_from_all, no_rename));
 }
 
@@ -94,7 +101,7 @@ void cmInstallProgramsCommand::FinalPass()
  * present in the build tree.  If a full path is given, it is just
  * returned.
  */
-std::string cmInstallProgramsCommand::FindInstallSource(const char* name) const
+static std::string FindInstallSource(cmMakefile& makefile, const char* name)
 {
   if (cmSystemTools::FileIsFullPath(name) ||
       cmGeneratorExpression::Find(name) == 0) {
@@ -103,10 +110,10 @@ std::string cmInstallProgramsCommand::FindInstallSource(const char* name) const
   }
 
   // This is a relative path.
-  std::string tb = this->Makefile->GetCurrentBinaryDirectory();
+  std::string tb = makefile.GetCurrentBinaryDirectory();
   tb += "/";
   tb += name;
-  std::string ts = this->Makefile->GetCurrentSourceDirectory();
+  std::string ts = makefile.GetCurrentSourceDirectory();
   ts += "/";
   ts += name;
 

Source/cmInstallProgramsCommand.h

@@ -37,24 +37,6 @@ public:
    */
   bool InitialPass(std::vector<std::string> const& args,
                    cmExecutionStatus& status) override;
-
-  /**
-   * This is called at the end after all the information
-   * specified by the command is accumulated. Most commands do
-   * not implement this method.  At this point, reading and
-   * writing to the cache can be done.
-   */
-  void FinalPass() override;
-
-  bool HasFinalPass() const override { return true; }
-
-protected:
-  std::string FindInstallSource(const char* name) const;
-
-private:
-  std::vector<std::string> FinalArgs;
-  std::string Destination;
-  std::vector<std::string> Files;
 };
 
 #endif

Source/cmLoadCommandCommand.cxx

@@ -25,21 +25,89 @@ class cmExecutionStatus;
 #  include <malloc.h> /* for malloc/free on QNX */
 #endif
 
-extern "C" void TrapsForSignalsCFunction(int sig);
+namespace {
+
+const char* LastName = nullptr;
+
+extern "C" void TrapsForSignals(int sig)
+{
+  fprintf(stderr, "CMake loaded command %s crashed with signal: %d.\n",
+          LastName, sig);
+}
+
+struct SignalHandlerGuard
+{
+  explicit SignalHandlerGuard(const char* name)
+  {
+    LastName = name != nullptr ? name : "????";
+
+    signal(SIGSEGV, TrapsForSignals);
+#ifdef SIGBUS
+    signal(SIGBUS, TrapsForSignals);
+#endif
+    signal(SIGILL, TrapsForSignals);
+  }
+
+  ~SignalHandlerGuard()
+  {
+    signal(SIGSEGV, nullptr);
+#ifdef SIGBUS
+    signal(SIGBUS, nullptr);
+#endif
+    signal(SIGILL, nullptr);
+  }
+
+  SignalHandlerGuard(SignalHandlerGuard const&) = delete;
+  SignalHandlerGuard& operator=(SignalHandlerGuard const&) = delete;
+};
+
+struct LoadedCommandImpl : cmLoadedCommandInfo
+{
+  explicit LoadedCommandImpl(CM_INIT_FUNCTION init)
+    : cmLoadedCommandInfo{ 0,       0,       &cmStaticCAPI, 0,
+                           nullptr, nullptr, nullptr,       nullptr,
+                           nullptr, nullptr, nullptr,       nullptr }
+  {
+    init(this);
+  }
+
+  ~LoadedCommandImpl()
+  {
+    if (this->Destructor) {
+      SignalHandlerGuard guard(this->Name);
+      this->Destructor(this);
+    }
+    if (this->Error != nullptr) {
+      free(this->Error);
+    }
+  }
+
+  LoadedCommandImpl(LoadedCommandImpl const&) = delete;
+  LoadedCommandImpl& operator=(LoadedCommandImpl const&) = delete;
+
+  int DoInitialPass(cmMakefile* mf, int argc, char* argv[])
+  {
+    SignalHandlerGuard guard(this->Name);
+    return this->InitialPass(this, mf, argc, argv);
+  }
+
+  void DoFinalPass(cmMakefile* mf)
+  {
+    SignalHandlerGuard guard(this->Name);
+    this->FinalPass(this, mf);
+  }
+};
 
 // a class for loadabple commands
 class cmLoadedCommand : public cmCommand
 {
 public:
-  cmLoadedCommand()
+  cmLoadedCommand() = default;
+  explicit cmLoadedCommand(CM_INIT_FUNCTION init)
+    : Impl(std::make_shared<LoadedCommandImpl>(init))
   {
-    memset(&this->info, 0, sizeof(this->info));
-    this->info.CAPI = &cmStaticCAPI;
   }
 
-  //! clean up any memory allocated by the plugin
-  ~cmLoadedCommand() override;
-
   /**
    * This is a virtual constructor for the command.
    */
@@ -47,8 +115,8 @@ public:
   {
     auto newC = cm::make_unique<cmLoadedCommand>();
     // we must copy when we clone
-    memcpy(&newC->info, &this->info, sizeof(info));
-    return std::unique_ptr<cmLoadedCommand>(std::move(newC));
+    newC->Impl = this->Impl;
+    return std::unique_ptr<cmCommand>(std::move(newC));
   }
 
   /**
@@ -58,66 +126,20 @@ public:
   bool InitialPass(std::vector<std::string> const& args,
                    cmExecutionStatus&) override;
 
-  /**
-   * This is called at the end after all the information
-   * specified by the command is accumulated. Most commands do
-   * not implement this method.  At this point, reading and
-   * writing to the cache can be done.
-   */
-  void FinalPass() override;
-  bool HasFinalPass() const override
-  {
-    return this->info.FinalPass != nullptr;
-  }
-
-  static const char* LastName;
-  static void TrapsForSignals(int sig)
-  {
-    fprintf(stderr, "CMake loaded command %s crashed with signal: %d.\n",
-            cmLoadedCommand::LastName, sig);
-  }
-  static void InstallSignalHandlers(const char* name, int remove = 0)
-  {
-    cmLoadedCommand::LastName = name;
-    if (!name) {
-      cmLoadedCommand::LastName = "????";
-    }
-
-    if (!remove) {
-      signal(SIGSEGV, TrapsForSignalsCFunction);
-#ifdef SIGBUS
-      signal(SIGBUS, TrapsForSignalsCFunction);
-#endif
-      signal(SIGILL, TrapsForSignalsCFunction);
-    } else {
-      signal(SIGSEGV, nullptr);
-#ifdef SIGBUS
-      signal(SIGBUS, nullptr);
-#endif
-      signal(SIGILL, nullptr);
-    }
-  }
-
-  cmLoadedCommandInfo info;
+private:
+  std::shared_ptr<LoadedCommandImpl> Impl;
 };
 
-extern "C" void TrapsForSignalsCFunction(int sig)
-{
-  cmLoadedCommand::TrapsForSignals(sig);
-}
-
-const char* cmLoadedCommand::LastName = nullptr;
-
 bool cmLoadedCommand::InitialPass(std::vector<std::string> const& args,
                                   cmExecutionStatus&)
 {
-  if (!info.InitialPass) {
+  if (!this->Impl->InitialPass) {
     return true;
   }
 
   // clear the error string
-  if (this->info.Error) {
-    free(this->info.Error);
+  if (this->Impl->Error) {
+    free(this->Impl->Error);
   }
 
   // create argc and argv and then invoke the command
@@ -130,42 +152,26 @@ bool cmLoadedCommand::InitialPass(std::vector<std::string> const& args,
   for (i = 0; i < argc; ++i) {
     argv[i] = strdup(args[i].c_str());
   }
-  cmLoadedCommand::InstallSignalHandlers(info.Name);
-  int result = info.InitialPass(&info, this->Makefile, argc, argv);
-  cmLoadedCommand::InstallSignalHandlers(info.Name, 1);
+  int result = this->Impl->DoInitialPass(this->Makefile, argc, argv);
   cmFreeArguments(argc, argv);
 
   if (result) {
+    if (this->Impl->FinalPass) {
+      auto impl = this->Impl;
+      this->Makefile->AddFinalAction(
+        [impl](cmMakefile& makefile) { impl->DoFinalPass(&makefile); });
+    }
     return true;
   }
 
   /* Initial Pass must have failed so set the error string */
-  if (this->info.Error) {
-    this->SetError(this->info.Error);
+  if (this->Impl->Error) {
+    this->SetError(this->Impl->Error);
   }
   return false;
 }
 
-void cmLoadedCommand::FinalPass()
-{
-  if (this->info.FinalPass) {
-    cmLoadedCommand::InstallSignalHandlers(info.Name);
-    this->info.FinalPass(&this->info, this->Makefile);
-    cmLoadedCommand::InstallSignalHandlers(info.Name, 1);
-  }
-}
-
-cmLoadedCommand::~cmLoadedCommand()
-{
-  if (this->info.Destructor) {
-    cmLoadedCommand::InstallSignalHandlers(info.Name);
-    this->info.Destructor(&this->info);
-    cmLoadedCommand::InstallSignalHandlers(info.Name, 1);
-  }
-  if (this->info.Error) {
-    free(this->info.Error);
-  }
-}
+} // namespace
 
 // cmLoadCommandCommand
 bool cmLoadCommandCommand::InitialPass(std::vector<std::string> const& args,
@@ -240,10 +246,8 @@ bool cmLoadCommandCommand::InitialPass(std::vector<std::string> const& args,
   // if the symbol is found call it to set the name on the
   // function blocker
   if (initFunction) {
-    // create a function blocker and set it up
-    auto f = cm::make_unique<cmLoadedCommand>();
-    (*initFunction)(&f->info);
-    this->Makefile->GetState()->AddScriptedCommand(args[0], std::move(f));
+    this->Makefile->GetState()->AddScriptedCommand(
+      args[0], cm::make_unique<cmLoadedCommand>(initFunction));
     return true;
   }
   this->SetError("Attempt to load command failed. "

Source/cmMakefile.cxx

@@ -414,9 +414,6 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
         if (this->GetCMakeInstance()->GetWorkingMode() != cmake::NORMAL_MODE) {
           cmSystemTools::SetFatalErrorOccured();
         }
-      } else if (pcmd->HasFinalPass()) {
-        // use the command
-        this->FinalPassCommands.push_back(std::move(pcmd));
       }
     }
   } else {
@@ -767,6 +764,11 @@ struct file_not_persistent
 };
 }
 
+void cmMakefile::AddFinalAction(FinalAction action)
+{
+  this->FinalActions.push_back(std::move(action));
+}
+
 void cmMakefile::FinalPass()
 {
   // do all the variable expansions here
@@ -774,8 +776,8 @@ void cmMakefile::FinalPass()
 
   // give all the commands a chance to do something
   // after the file has been parsed before generation
-  for (auto& command : this->FinalPassCommands) {
-    command->FinalPass();
+  for (FinalAction& action : this->FinalActions) {
+    action(*this);
   }
 
   // go through all configured files and see which ones still exist.

Source/cmMakefile.h

@@ -7,6 +7,7 @@
 
 #include "cmsys/RegularExpression.hxx"
 #include <deque>
+#include <functional>
 #include <map>
 #include <memory>
 #include <set>
@@ -31,7 +32,6 @@
 #  include "cmSourceGroup.h"
 #endif
 
-class cmCommand;
 class cmCompiledGeneratorExpression;
 class cmCustomCommandLines;
 class cmExecutionStatus;
@@ -125,6 +125,13 @@ public:
   bool EnforceUniqueName(std::string const& name, std::string& msg,
                          bool isCustom = false) const;
 
+  using FinalAction = std::function<void(cmMakefile&)>;
+
+  /**
+   * Register an action that is executed during FinalPass
+   */
+  void AddFinalAction(FinalAction action);
+
   /**
    * Perform FinalPass, Library dependency analysis etc before output of the
    * makefile.
@@ -132,7 +139,7 @@ public:
   void ConfigureFinalPass();
 
   /**
-   * run the final pass on all commands.
+   * run all FinalActions.
    */
   void FinalPass();
 
@@ -937,7 +944,7 @@ protected:
   size_t ObjectLibrariesSourceGroupIndex;
 #endif
 
-  std::vector<std::unique_ptr<cmCommand>> FinalPassCommands;
+  std::vector<FinalAction> FinalActions;
   cmGlobalGenerator* GlobalGenerator;
   bool IsFunctionBlocked(const cmListFileFunction& lff,
                          cmExecutionStatus& status);

Source/cmVariableWatchCommand.cxx

@@ -3,6 +3,7 @@
 #include "cmVariableWatchCommand.h"
 
 #include <sstream>
+#include <utility>
 
 #include "cmExecutionStatus.h"
 #include "cmListFileCache.h"
@@ -84,15 +85,39 @@ static void deleteVariableWatchCallbackData(void* client_data)
   delete data;
 }
 
-cmVariableWatchCommand::cmVariableWatchCommand() = default;
-
-cmVariableWatchCommand::~cmVariableWatchCommand()
+/** This command does not really have a final pass but it needs to
+    stay alive since it owns variable watch callback information. */
+class FinalAction
 {
-  for (std::string const& wv : this->WatchedVariables) {
-    this->Makefile->GetCMakeInstance()->GetVariableWatch()->RemoveWatch(
-      wv, cmVariableWatchCommandVariableAccessed);
+public:
+  FinalAction(cmMakefile* makefile, std::string variable)
+    : Action(std::make_shared<Impl>(makefile, std::move(variable)))
+  {
   }
-}
+
+  void operator()(cmMakefile&) const {}
+
+private:
+  struct Impl
+  {
+    Impl(cmMakefile* makefile, std::string variable)
+      : Makefile(makefile)
+      , Variable(std::move(variable))
+    {
+    }
+
+    ~Impl()
+    {
+      this->Makefile->GetCMakeInstance()->GetVariableWatch()->RemoveWatch(
+        this->Variable, cmVariableWatchCommandVariableAccessed);
+    }
+
+    cmMakefile* Makefile;
+    std::string Variable;
+  };
+
+  std::shared_ptr<Impl const> Action;
+};
 
 bool cmVariableWatchCommand::InitialPass(std::vector<std::string> const& args,
                                          cmExecutionStatus&)
@@ -118,7 +143,6 @@ bool cmVariableWatchCommand::InitialPass(std::vector<std::string> const& args,
   data->InCallback = false;
   data->Command = command;
 
-  this->WatchedVariables.insert(variable);
   if (!this->Makefile->GetCMakeInstance()->GetVariableWatch()->AddWatch(
         variable, cmVariableWatchCommandVariableAccessed, data,
         deleteVariableWatchCallbackData)) {
@@ -126,5 +150,6 @@ bool cmVariableWatchCommand::InitialPass(std::vector<std::string> const& args,
     return false;
   }
 
+  this->Makefile->AddFinalAction(FinalAction(this->Makefile, variable));
   return true;
 }

Source/cmVariableWatchCommand.h

@@ -5,7 +5,6 @@
 
 #include "cmConfigure.h" // IWYU pragma: keep
 
-#include <set>
 #include <string>
 #include <vector>
 
@@ -30,25 +29,12 @@ public:
     return cm::make_unique<cmVariableWatchCommand>();
   }
 
-  //! Default constructor
-  cmVariableWatchCommand();
-
-  //! Destructor.
-  ~cmVariableWatchCommand() override;
-
   /**
    * This is called when the command is first encountered in
    * the CMakeLists.txt file.
    */
   bool InitialPass(std::vector<std::string> const& args,
                    cmExecutionStatus& status) override;
-
-  /** This command does not really have a final pass but it needs to
-      stay alive since it owns variable watch callback information. */
-  bool HasFinalPass() const override { return true; }
-
-protected:
-  std::set<std::string> WatchedVariables;
 };
 
 #endif