Merge topic 'control-block3'

41364824ad cmFunctionBlocker: Recycle functions
6491270e0d cmFunctionBlocker: Move check for matching args
af24e4ef6e cmFunctionBlocker: Move common logic to base
ef38ff22f7 cm*FunctionBlocker: Extract function Replay
b51fba6298 cmMakefile: Add OnExecuteCommand callback
c76500949d cm*FunctionBlocker: Move to source file

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

Source/CMakeLists.txt

@@ -532,6 +532,8 @@ set(SRCS
   cmFindProgramCommand.h
   cmForEachCommand.cxx
   cmForEachCommand.h
+  cmFunctionBlocker.cxx
+  cmFunctionBlocker.h
   cmFunctionCommand.cxx
   cmFunctionCommand.h
   cmGetCMakePropertyCommand.cxx

Source/CTest/cmCTestScriptHandler.cxx

@@ -24,7 +24,6 @@
 #include "cmCTestUploadCommand.h"
 #include "cmCommand.h"
 #include "cmDuration.h"
-#include "cmFunctionBlocker.h"
 #include "cmGeneratedFileStream.h"
 #include "cmGlobalGenerator.h"
 #include "cmMakefile.h"
@@ -49,32 +48,8 @@
 #  include <unistd.h>
 #endif
 
-class cmExecutionStatus;
-struct cmListFileFunction;
-
 #define CTEST_INITIAL_CMAKE_OUTPUT_FILE_NAME "CTestInitialCMakeOutput.log"
 
-// used to keep elapsed time up to date
-class cmCTestScriptFunctionBlocker : public cmFunctionBlocker
-{
-public:
-  bool IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile& mf,
-                         cmExecutionStatus& /*status*/) override;
-  // virtual bool ShouldRemove(const cmListFileFunction& lff, cmMakefile &mf);
-  // virtual void ScopeEnded(cmMakefile &mf);
-
-  cmCTestScriptHandler* CTestScriptHandler;
-};
-
-// simply update the time and don't block anything
-bool cmCTestScriptFunctionBlocker::IsFunctionBlocked(
-  const cmListFileFunction& /*lff*/, cmMakefile& /*mf*/,
-  cmExecutionStatus& /*status*/)
-{
-  this->CTestScriptHandler->UpdateElapsedTime();
-  return false;
-}
-
 cmCTestScriptHandler::cmCTestScriptHandler()
 {
   this->Backup = false;
@@ -373,12 +348,8 @@ int cmCTestScriptHandler::ReadInScript(const std::string& total_script_arg)
   this->Makefile->AddDefinition("CMAKE_LEGACY_CYGWIN_WIN32", "0");
 #endif
 
-  // always add a function blocker to update the elapsed time
-  {
-    auto fb = cm::make_unique<cmCTestScriptFunctionBlocker>();
-    fb->CTestScriptHandler = this;
-    this->Makefile->AddFunctionBlocker(std::move(fb));
-  }
+  // set a callback function to update the elapsed time
+  this->Makefile->OnExecuteCommand([this] { this->UpdateElapsedTime(); });
 
   /* Execute CTestScriptMode.cmake, which loads CMakeDetermineSystem and
   CMakeSystemSpecificInformation, so

Source/cmForEachCommand.cxx

@@ -8,16 +8,40 @@
 #include <utility>
 
 #include "cm_memory.hxx"
+#include "cm_static_string_view.hxx"
+#include "cm_string_view.hxx"
 
 #include "cmExecutionStatus.h"
+#include "cmFunctionBlocker.h"
+#include "cmListFileCache.h"
 #include "cmMakefile.h"
 #include "cmMessageType.h"
 #include "cmRange.h"
 #include "cmSystemTools.h"
 
+class cmForEachFunctionBlocker : public cmFunctionBlocker
+{
+public:
+  cmForEachFunctionBlocker(cmMakefile* mf);
+  ~cmForEachFunctionBlocker() override;
+
+  cm::string_view StartCommandName() const override { return "foreach"_s; }
+  cm::string_view EndCommandName() const override { return "endforeach"_s; }
+
+  bool ArgumentsMatch(cmListFileFunction const& lff,
+                      cmMakefile& mf) const override;
+
+  bool Replay(std::vector<cmListFileFunction> functions,
+              cmExecutionStatus& inStatus) override;
+
+  std::vector<std::string> Args;
+
+private:
+  cmMakefile* Makefile;
+};
+
 cmForEachFunctionBlocker::cmForEachFunctionBlocker(cmMakefile* mf)
   : Makefile(mf)
-  , Depth(0)
 {
   this->Makefile->PushLoopBlock();
 }
@@ -27,89 +51,58 @@ cmForEachFunctionBlocker::~cmForEachFunctionBlocker()
   this->Makefile->PopLoopBlock();
 }
 
-bool cmForEachFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
-                                                 cmMakefile& mf,
-                                                 cmExecutionStatus& inStatus)
+bool cmForEachFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
+                                              cmMakefile& mf) const
 {
-  if (lff.Name.Lower == "foreach") {
-    // record the number of nested foreach commands
-    this->Depth++;
-  } else if (lff.Name.Lower == "endforeach") {
-    // if this is the endofreach for this statement
-    if (!this->Depth) {
-      // Remove the function blocker for this scope or bail.
-      std::unique_ptr<cmFunctionBlocker> fb(
-        mf.RemoveFunctionBlocker(this, lff));
-      if (!fb) {
-        return false;
-      }
+  std::vector<std::string> expandedArguments;
+  mf.ExpandArguments(lff.Arguments, expandedArguments);
+  return expandedArguments.empty() || expandedArguments[0] == this->Args[0];
+}
 
-      // at end of for each execute recorded commands
-      // store the old value
-      std::string oldDef;
-      if (mf.GetDefinition(this->Args[0])) {
-        oldDef = mf.GetDefinition(this->Args[0]);
-      }
+bool cmForEachFunctionBlocker::Replay(
+  std::vector<cmListFileFunction> functions, cmExecutionStatus& inStatus)
+{
+  cmMakefile& mf = inStatus.GetMakefile();
+  // at end of for each execute recorded commands
+  // store the old value
+  std::string oldDef;
+  if (mf.GetDefinition(this->Args[0])) {
+    oldDef = mf.GetDefinition(this->Args[0]);
+  }
 
-      for (std::string const& arg : cmMakeRange(this->Args).advance(1)) {
-        // set the variable to the loop value
-        mf.AddDefinition(this->Args[0], arg);
-        // Invoke all the functions that were collected in the block.
-        cmExecutionStatus status(mf);
-        for (cmListFileFunction const& func : this->Functions) {
-          status.Clear();
-          mf.ExecuteCommand(func, status);
-          if (status.GetReturnInvoked()) {
-            inStatus.SetReturnInvoked();
-            // restore the variable to its prior value
-            mf.AddDefinition(this->Args[0], oldDef);
-            return true;
-          }
-          if (status.GetBreakInvoked()) {
-            // restore the variable to its prior value
-            mf.AddDefinition(this->Args[0], oldDef);
-            return true;
-          }
-          if (status.GetContinueInvoked()) {
-            break;
-          }
-          if (cmSystemTools::GetFatalErrorOccured()) {
-            return true;
-          }
-        }
+  for (std::string const& arg : cmMakeRange(this->Args).advance(1)) {
+    // set the variable to the loop value
+    mf.AddDefinition(this->Args[0], arg);
+    // Invoke all the functions that were collected in the block.
+    cmExecutionStatus status(mf);
+    for (cmListFileFunction const& func : functions) {
+      status.Clear();
+      mf.ExecuteCommand(func, status);
+      if (status.GetReturnInvoked()) {
+        inStatus.SetReturnInvoked();
+        // restore the variable to its prior value
+        mf.AddDefinition(this->Args[0], oldDef);
+        return true;
+      }
+      if (status.GetBreakInvoked()) {
+        // restore the variable to its prior value
+        mf.AddDefinition(this->Args[0], oldDef);
+        return true;
+      }
+      if (status.GetContinueInvoked()) {
+        break;
+      }
+      if (cmSystemTools::GetFatalErrorOccured()) {
+        return true;
       }
-
-      // restore the variable to its prior value
-      mf.AddDefinition(this->Args[0], oldDef);
-      return true;
     }
-    // close out a nested foreach
-    this->Depth--;
   }
 
-  // record the command
-  this->Functions.push_back(lff);
-
-  // always return true
+  // restore the variable to its prior value
+  mf.AddDefinition(this->Args[0], oldDef);
   return true;
 }
 
-bool cmForEachFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
-                                            cmMakefile& mf)
-{
-  if (lff.Name.Lower == "endforeach") {
-    std::vector<std::string> expandedArguments;
-    mf.ExpandArguments(lff.Arguments, expandedArguments);
-    // if the endforeach has arguments then make sure
-    // they match the begin foreach arguments
-    if ((expandedArguments.empty() ||
-         (expandedArguments[0] == this->Args[0]))) {
-      return true;
-    }
-  }
-  return false;
-}
-
 bool cmForEachCommand::InitialPass(std::vector<std::string> const& args,
                                    cmExecutionStatus&)
 {

Source/cmForEachCommand.h

@@ -11,28 +11,8 @@
 #include "cm_memory.hxx"
 
 #include "cmCommand.h"
-#include "cmFunctionBlocker.h"
-#include "cmListFileCache.h"
 
 class cmExecutionStatus;
-class cmMakefile;
-
-class cmForEachFunctionBlocker : public cmFunctionBlocker
-{
-public:
-  cmForEachFunctionBlocker(cmMakefile* mf);
-  ~cmForEachFunctionBlocker() override;
-  bool IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile& mf,
-                         cmExecutionStatus&) override;
-  bool ShouldRemove(const cmListFileFunction& lff, cmMakefile& mf) override;
-
-  std::vector<std::string> Args;
-  std::vector<cmListFileFunction> Functions;
-
-private:
-  cmMakefile* Makefile;
-  int Depth;
-};
 
 /// Starts foreach() ... endforeach() block
 class cmForEachCommand : public cmCommand

Source/cmFunctionBlocker.cxx

@@ -0,0 +1,46 @@
+/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
+   file Copyright.txt or https://cmake.org/licensing for details.  */
+#include "cmFunctionBlocker.h"
+
+#include <cassert>
+#include <sstream>
+#include <utility>
+
+#include "cmExecutionStatus.h"
+#include "cmMakefile.h"
+#include "cmMessageType.h"
+
+bool cmFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
+                                          cmExecutionStatus& status)
+{
+  if (lff.Name.Lower == this->StartCommandName()) {
+    this->ScopeDepth++;
+  } else if (lff.Name.Lower == this->EndCommandName()) {
+    this->ScopeDepth--;
+    if (this->ScopeDepth == 0U) {
+      cmMakefile& mf = status.GetMakefile();
+      auto self = mf.RemoveFunctionBlocker();
+      assert(self.get() == this);
+
+      if (!this->ArgumentsMatch(lff, mf)) {
+        cmListFileContext const& lfc = this->GetStartingContext();
+        cmListFileContext closingContext =
+          cmListFileContext::FromCommandContext(lff, lfc.FilePath);
+        std::ostringstream e;
+        /* clang-format off */
+        e << "A logical block opening on the line\n"
+          << "  " << lfc << "\n"
+          << "closes on the line\n"
+          << "  " << closingContext << "\n"
+          << "with mis-matching arguments.";
+        /* clang-format on */
+        mf.IssueMessage(MessageType::AUTHOR_WARNING, e.str());
+      }
+
+      return this->Replay(std::move(this->Functions), status);
+    }
+  }
+
+  this->Functions.push_back(lff);
+  return true;
+}

Source/cmFunctionBlocker.h

@@ -3,6 +3,12 @@
 #ifndef cmFunctionBlocker_h
 #define cmFunctionBlocker_h
 
+#include "cmConfigure.h" // IWYU pragma: keep
+
+#include <vector>
+
+#include "cm_string_view.hxx"
+
 #include "cmListFileCache.h"
 
 class cmExecutionStatus;
@@ -14,17 +20,8 @@ public:
   /**
    * should a function be blocked
    */
-  virtual bool IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile& mf,
-                                 cmExecutionStatus& status) = 0;
-
-  /**
-   * should this function blocker be removed, useful when one function adds a
-   * blocker and another must remove it
-   */
-  virtual bool ShouldRemove(const cmListFileFunction&, cmMakefile&)
-  {
-    return false;
-  }
+  bool IsFunctionBlocked(cmListFileFunction const& lff,
+                         cmExecutionStatus& status);
 
   virtual ~cmFunctionBlocker() = default;
 
@@ -38,8 +35,20 @@ public:
     return this->StartingContext;
   }
 
+private:
+  virtual cm::string_view StartCommandName() const = 0;
+  virtual cm::string_view EndCommandName() const = 0;
+
+  virtual bool ArgumentsMatch(cmListFileFunction const& lff,
+                              cmMakefile& mf) const = 0;
+
+  virtual bool Replay(std::vector<cmListFileFunction> functions,
+                      cmExecutionStatus& status) = 0;
+
 private:
   cmListFileContext StartingContext;
+  std::vector<cmListFileFunction> Functions;
+  unsigned int ScopeDepth = 1;
 };
 
 #endif

Source/cmFunctionCommand.cxx

@@ -5,8 +5,13 @@
 #include <sstream>
 #include <utility>
 
+#include "cm_static_string_view.hxx"
+#include "cm_string_view.hxx"
+
 #include "cmAlgorithms.h"
 #include "cmExecutionStatus.h"
+#include "cmFunctionBlocker.h"
+#include "cmListFileCache.h"
 #include "cmMakefile.h"
 #include "cmPolicies.h"
 #include "cmRange.h"
@@ -102,53 +107,42 @@ bool cmFunctionHelperCommand::operator()(
   return true;
 }
 
-bool cmFunctionFunctionBlocker::IsFunctionBlocked(
-  const cmListFileFunction& lff, cmMakefile& mf, cmExecutionStatus&)
+class cmFunctionFunctionBlocker : public cmFunctionBlocker
 {
-  // record commands until we hit the ENDFUNCTION
-  // at the ENDFUNCTION call we shift gears and start looking for invocations
-  if (lff.Name.Lower == "function") {
-    this->Depth++;
-  } else if (lff.Name.Lower == "endfunction") {
-    // if this is the endfunction for this function then execute
-    if (!this->Depth) {
-      // create a new command and add it to cmake
-      cmFunctionHelperCommand f;
-      f.Args = this->Args;
-      f.Functions = this->Functions;
-      f.FilePath = this->GetStartingContext().FilePath;
-      mf.RecordPolicies(f.Policies);
-      mf.GetState()->AddScriptedCommand(this->Args[0], std::move(f));
-      // remove the function blocker now that the function is defined
-      mf.RemoveFunctionBlocker(this, lff);
-      return true;
-    }
-    // decrement for each nested function that ends
-    this->Depth--;
-  }
+public:
+  cm::string_view StartCommandName() const override { return "function"_s; }
+  cm::string_view EndCommandName() const override { return "endfunction"_s; }
 
-  // if it wasn't an endfunction and we are not executing then we must be
-  // recording
-  this->Functions.push_back(lff);
-  return true;
-}
+  bool ArgumentsMatch(cmListFileFunction const&,
+                      cmMakefile& mf) const override;
 
-bool cmFunctionFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
-                                             cmMakefile& mf)
+  bool Replay(std::vector<cmListFileFunction> functions,
+              cmExecutionStatus& status) override;
+
+  std::vector<std::string> Args;
+};
+
+bool cmFunctionFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
+                                               cmMakefile& mf) const
 {
-  if (lff.Name.Lower == "endfunction") {
-    std::vector<std::string> expandedArguments;
-    mf.ExpandArguments(lff.Arguments, expandedArguments,
-                       this->GetStartingContext().FilePath.c_str());
-    // if the endfunction has arguments then make sure
-    // they match the ones in the opening function command
-    if ((expandedArguments.empty() ||
-         (expandedArguments[0] == this->Args[0]))) {
-      return true;
-    }
-  }
+  std::vector<std::string> expandedArguments;
+  mf.ExpandArguments(lff.Arguments, expandedArguments,
+                     this->GetStartingContext().FilePath.c_str());
+  return expandedArguments.empty() || expandedArguments[0] == this->Args[0];
+}
 
-  return false;
+bool cmFunctionFunctionBlocker::Replay(
+  std::vector<cmListFileFunction> functions, cmExecutionStatus& status)
+{
+  cmMakefile& mf = status.GetMakefile();
+  // create a new command and add it to cmake
+  cmFunctionHelperCommand f;
+  f.Args = this->Args;
+  f.Functions = std::move(functions);
+  f.FilePath = this->GetStartingContext().FilePath;
+  mf.RecordPolicies(f.Policies);
+  mf.GetState()->AddScriptedCommand(this->Args[0], std::move(f));
+  return true;
 }
 
 bool cmFunctionCommand::InitialPass(std::vector<std::string> const& args,

Source/cmFunctionCommand.h

@@ -11,23 +11,8 @@
 #include "cm_memory.hxx"
 
 #include "cmCommand.h"
-#include "cmFunctionBlocker.h"
-#include "cmListFileCache.h"
 
 class cmExecutionStatus;
-class cmMakefile;
-
-class cmFunctionFunctionBlocker : public cmFunctionBlocker
-{
-public:
-  bool IsFunctionBlocked(const cmListFileFunction&, cmMakefile& mf,
-                         cmExecutionStatus&) override;
-  bool ShouldRemove(const cmListFileFunction&, cmMakefile& mf) override;
-
-  std::vector<std::string> Args;
-  std::vector<cmListFileFunction> Functions;
-  int Depth = 0;
-};
 
 /// Starts function() ... endfunction() block
 class cmFunctionCommand : public cmCommand

Source/cmIfCommand.cxx

@@ -3,10 +3,14 @@
 #include "cmIfCommand.h"
 
 #include "cm_memory.hxx"
+#include "cm_static_string_view.hxx"
+#include "cm_string_view.hxx"
 
 #include "cmConditionEvaluator.h"
 #include "cmExecutionStatus.h"
 #include "cmExpandedCommandArgument.h"
+#include "cmFunctionBlocker.h"
+#include "cmListFileCache.h"
 #include "cmMakefile.h"
 #include "cmMessageType.h"
 #include "cmOutputConverter.h"
@@ -28,152 +32,138 @@ static std::string cmIfCommandError(
   return err;
 }
 
-//=========================================================================
-bool cmIfFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
-                                            cmMakefile& mf,
-                                            cmExecutionStatus& inStatus)
+class cmIfFunctionBlocker : public cmFunctionBlocker
 {
-  // we start by recording all the functions
-  if (lff.Name.Lower == "if") {
-    this->ScopeDepth++;
-  } else if (lff.Name.Lower == "endif") {
-    this->ScopeDepth--;
-    // if this is the endif for this if statement, then start executing
-    if (!this->ScopeDepth) {
-      // Remove the function blocker for this scope or bail.
-      std::unique_ptr<cmFunctionBlocker> fb(
-        mf.RemoveFunctionBlocker(this, lff));
-      if (!fb) {
-        return false;
+public:
+  cm::string_view StartCommandName() const override { return "if"_s; }
+  cm::string_view EndCommandName() const override { return "endif"_s; }
+
+  bool ArgumentsMatch(cmListFileFunction const& lff,
+                      cmMakefile&) const override;
+
+  bool Replay(std::vector<cmListFileFunction> functions,
+              cmExecutionStatus& inStatus) override;
+
+  std::vector<cmListFileArgument> Args;
+  bool IsBlocking;
+  bool HasRun = false;
+  bool ElseSeen = false;
+};
+
+bool cmIfFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
+                                         cmMakefile&) const
+{
+  return lff.Arguments.empty() || lff.Arguments == this->Args;
+}
+
+bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
+                                 cmExecutionStatus& inStatus)
+{
+  cmMakefile& mf = inStatus.GetMakefile();
+  // execute the functions for the true parts of the if statement
+  cmExecutionStatus status(mf);
+  int scopeDepth = 0;
+  for (cmListFileFunction const& func : functions) {
+    // keep track of scope depth
+    if (func.Name.Lower == "if") {
+      scopeDepth++;
+    }
+    if (func.Name.Lower == "endif") {
+      scopeDepth--;
+    }
+    // watch for our state change
+    if (scopeDepth == 0 && func.Name.Lower == "else") {
+
+      if (this->ElseSeen) {
+        cmListFileBacktrace bt = mf.GetBacktrace(func);
+        mf.GetCMakeInstance()->IssueMessage(
+          MessageType::FATAL_ERROR,
+          "A duplicate ELSE command was found inside an IF block.", bt);
+        cmSystemTools::SetFatalErrorOccured();
+        return true;
       }
 
-      // execute the functions for the true parts of the if statement
-      cmExecutionStatus status(mf);
-      int scopeDepth = 0;
-      for (cmListFileFunction const& func : this->Functions) {
-        // keep track of scope depth
-        if (func.Name.Lower == "if") {
-          scopeDepth++;
-        }
-        if (func.Name.Lower == "endif") {
-          scopeDepth--;
+      this->IsBlocking = this->HasRun;
+      this->HasRun = true;
+      this->ElseSeen = true;
+
+      // if trace is enabled, print a (trivially) evaluated "else"
+      // statement
+      if (!this->IsBlocking && mf.GetCMakeInstance()->GetTrace()) {
+        mf.PrintCommandTrace(func);
+      }
+    } else if (scopeDepth == 0 && func.Name.Lower == "elseif") {
+      if (this->ElseSeen) {
+        cmListFileBacktrace bt = mf.GetBacktrace(func);
+        mf.GetCMakeInstance()->IssueMessage(
+          MessageType::FATAL_ERROR,
+          "An ELSEIF command was found after an ELSE command.", bt);
+        cmSystemTools::SetFatalErrorOccured();
+        return true;
+      }
+
+      if (this->HasRun) {
+        this->IsBlocking = true;
+      } else {
+        // if trace is enabled, print the evaluated "elseif" statement
+        if (mf.GetCMakeInstance()->GetTrace()) {
+          mf.PrintCommandTrace(func);
         }
-        // watch for our state change
-        if (scopeDepth == 0 && func.Name.Lower == "else") {
-
-          if (this->ElseSeen) {
-            cmListFileBacktrace bt = mf.GetBacktrace(func);
-            mf.GetCMakeInstance()->IssueMessage(
-              MessageType::FATAL_ERROR,
-              "A duplicate ELSE command was found inside an IF block.", bt);
-            cmSystemTools::SetFatalErrorOccured();
-            return true;
-          }
 
-          this->IsBlocking = this->HasRun;
-          this->HasRun = true;
-          this->ElseSeen = true;
+        std::string errorString;
 
-          // if trace is enabled, print a (trivially) evaluated "else"
-          // statement
-          if (!this->IsBlocking && mf.GetCMakeInstance()->GetTrace()) {
-            mf.PrintCommandTrace(func);
-          }
-        } else if (scopeDepth == 0 && func.Name.Lower == "elseif") {
-          if (this->ElseSeen) {
-            cmListFileBacktrace bt = mf.GetBacktrace(func);
-            mf.GetCMakeInstance()->IssueMessage(
-              MessageType::FATAL_ERROR,
-              "An ELSEIF command was found after an ELSE command.", bt);
+        std::vector<cmExpandedCommandArgument> expandedArguments;
+        mf.ExpandArguments(func.Arguments, expandedArguments);
+
+        MessageType messType;
+
+        cmListFileContext conditionContext =
+          cmListFileContext::FromCommandContext(
+            func, this->GetStartingContext().FilePath);
+
+        cmConditionEvaluator conditionEvaluator(mf, conditionContext,
+                                                mf.GetBacktrace(func));
+
+        bool isTrue =
+          conditionEvaluator.IsTrue(expandedArguments, errorString, messType);
+
+        if (!errorString.empty()) {
+          std::string err = cmIfCommandError(expandedArguments);
+          err += errorString;
+          cmListFileBacktrace bt = mf.GetBacktrace(func);
+          mf.GetCMakeInstance()->IssueMessage(messType, err, bt);
+          if (messType == MessageType::FATAL_ERROR) {
             cmSystemTools::SetFatalErrorOccured();
             return true;
           }
-
-          if (this->HasRun) {
-            this->IsBlocking = true;
-          } else {
-            // if trace is enabled, print the evaluated "elseif" statement
-            if (mf.GetCMakeInstance()->GetTrace()) {
-              mf.PrintCommandTrace(func);
-            }
-
-            std::string errorString;
-
-            std::vector<cmExpandedCommandArgument> expandedArguments;
-            mf.ExpandArguments(func.Arguments, expandedArguments);
-
-            MessageType messType;
-
-            cmListFileContext conditionContext =
-              cmListFileContext::FromCommandContext(
-                func, this->GetStartingContext().FilePath);
-
-            cmConditionEvaluator conditionEvaluator(mf, conditionContext,
-                                                    mf.GetBacktrace(func));
-
-            bool isTrue = conditionEvaluator.IsTrue(expandedArguments,
-                                                    errorString, messType);
-
-            if (!errorString.empty()) {
-              std::string err = cmIfCommandError(expandedArguments);
-              err += errorString;
-              cmListFileBacktrace bt = mf.GetBacktrace(func);
-              mf.GetCMakeInstance()->IssueMessage(messType, err, bt);
-              if (messType == MessageType::FATAL_ERROR) {
-                cmSystemTools::SetFatalErrorOccured();
-                return true;
-              }
-            }
-
-            if (isTrue) {
-              this->IsBlocking = false;
-              this->HasRun = true;
-            }
-          }
         }
 
-        // should we execute?
-        else if (!this->IsBlocking) {
-          status.Clear();
-          mf.ExecuteCommand(func, status);
-          if (status.GetReturnInvoked()) {
-            inStatus.SetReturnInvoked();
-            return true;
-          }
-          if (status.GetBreakInvoked()) {
-            inStatus.SetBreakInvoked();
-            return true;
-          }
-          if (status.GetContinueInvoked()) {
-            inStatus.SetContinueInvoked();
-            return true;
-          }
+        if (isTrue) {
+          this->IsBlocking = false;
+          this->HasRun = true;
         }
       }
-      return true;
     }
-  }
 
-  // record the command
-  this->Functions.push_back(lff);
-
-  // always return true
-  return true;
-}
-
-//=========================================================================
-bool cmIfFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
-                                       cmMakefile&)
-{
-  if (lff.Name.Lower == "endif") {
-    // if the endif has arguments, then make sure
-    // they match the arguments of the matching if
-    if (lff.Arguments.empty() || lff.Arguments == this->Args) {
-      return true;
+    // should we execute?
+    else if (!this->IsBlocking) {
+      status.Clear();
+      mf.ExecuteCommand(func, status);
+      if (status.GetReturnInvoked()) {
+        inStatus.SetReturnInvoked();
+        return true;
+      }
+      if (status.GetBreakInvoked()) {
+        inStatus.SetBreakInvoked();
+        return true;
+      }
+      if (status.GetContinueInvoked()) {
+        inStatus.SetContinueInvoked();
+        return true;
+      }
     }
   }
-
-  return false;
+  return true;
 }
 
 //=========================================================================
@@ -208,7 +198,6 @@ bool cmIfCommand(std::vector<cmListFileArgument> const& args,
   {
     auto fb = cm::make_unique<cmIfFunctionBlocker>();
     // if is isn't true block the commands
-    fb->ScopeDepth = 1;
     fb->IsBlocking = !isTrue;
     if (isTrue) {
       fb->HasRun = true;

Source/cmIfCommand.h

@@ -7,26 +7,8 @@
 
 #include <vector>
 
-#include "cmFunctionBlocker.h"
-#include "cmListFileCache.h"
-
 class cmExecutionStatus;
-class cmMakefile;
-
-class cmIfFunctionBlocker : public cmFunctionBlocker
-{
-public:
-  bool IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile& mf,
-                         cmExecutionStatus&) override;
-  bool ShouldRemove(const cmListFileFunction& lff, cmMakefile& mf) override;
-
-  std::vector<cmListFileArgument> Args;
-  std::vector<cmListFileFunction> Functions;
-  bool IsBlocking;
-  bool HasRun = false;
-  bool ElseSeen = false;
-  unsigned int ScopeDepth = 0;
-};
+struct cmListFileArgument;
 
 /// Starts an if block
 bool cmIfCommand(std::vector<cmListFileArgument> const& args,

Source/cmMacroCommand.cxx

@@ -7,9 +7,13 @@
 #include <utility>
 
 #include "cm_memory.hxx"
+#include "cm_static_string_view.hxx"
+#include "cm_string_view.hxx"
 
 #include "cmAlgorithms.h"
 #include "cmExecutionStatus.h"
+#include "cmFunctionBlocker.h"
+#include "cmListFileCache.h"
 #include "cmMakefile.h"
 #include "cmPolicies.h"
 #include "cmRange.h"
@@ -136,55 +140,43 @@ bool cmMacroHelperCommand::operator()(
   return true;
 }
 
-bool cmMacroFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
-                                               cmMakefile& mf,
-                                               cmExecutionStatus&)
+class cmMacroFunctionBlocker : public cmFunctionBlocker
 {
-  // record commands until we hit the ENDMACRO
-  // at the ENDMACRO call we shift gears and start looking for invocations
-  if (lff.Name.Lower == "macro") {
-    this->Depth++;
-  } else if (lff.Name.Lower == "endmacro") {
-    // if this is the endmacro for this macro then execute
-    if (!this->Depth) {
-      mf.AppendProperty("MACROS", this->Args[0].c_str());
-      // create a new command and add it to cmake
-      cmMacroHelperCommand f;
-      f.Args = this->Args;
-      f.Functions = this->Functions;
-      f.FilePath = this->GetStartingContext().FilePath;
-      mf.RecordPolicies(f.Policies);
-      mf.GetState()->AddScriptedCommand(this->Args[0], std::move(f));
-      // remove the function blocker now that the macro is defined
-      mf.RemoveFunctionBlocker(this, lff);
-      return true;
-    }
-    // decrement for each nested macro that ends
-    this->Depth--;
-  }
+public:
+  cm::string_view StartCommandName() const override { return "macro"_s; }
+  cm::string_view EndCommandName() const override { return "endmacro"_s; }
 
-  // if it wasn't an endmacro and we are not executing then we must be
-  // recording
-  this->Functions.push_back(lff);
-  return true;
-}
+  bool ArgumentsMatch(cmListFileFunction const&,
+                      cmMakefile& mf) const override;
+
+  bool Replay(std::vector<cmListFileFunction> functions,
+              cmExecutionStatus& status) override;
+
+  std::vector<std::string> Args;
+};
 
-bool cmMacroFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
-                                          cmMakefile& mf)
+bool cmMacroFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
+                                            cmMakefile& mf) const
 {
-  if (lff.Name.Lower == "endmacro") {
-    std::vector<std::string> expandedArguments;
-    mf.ExpandArguments(lff.Arguments, expandedArguments,
-                       this->GetStartingContext().FilePath.c_str());
-    // if the endmacro has arguments make sure they
-    // match the arguments of the macro
-    if ((expandedArguments.empty() ||
-         (expandedArguments[0] == this->Args[0]))) {
-      return true;
-    }
-  }
+  std::vector<std::string> expandedArguments;
+  mf.ExpandArguments(lff.Arguments, expandedArguments,
+                     this->GetStartingContext().FilePath.c_str());
+  return expandedArguments.empty() || expandedArguments[0] == this->Args[0];
+}
 
-  return false;
+bool cmMacroFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
+                                    cmExecutionStatus& status)
+{
+  cmMakefile& mf = status.GetMakefile();
+  mf.AppendProperty("MACROS", this->Args[0].c_str());
+  // create a new command and add it to cmake
+  cmMacroHelperCommand f;
+  f.Args = this->Args;
+  f.Functions = std::move(functions);
+  f.FilePath = this->GetStartingContext().FilePath;
+  mf.RecordPolicies(f.Policies);
+  mf.GetState()->AddScriptedCommand(this->Args[0], std::move(f));
+  return true;
 }
 
 bool cmMacroCommand::InitialPass(std::vector<std::string> const& args,

Source/cmMacroCommand.h

@@ -11,23 +11,8 @@
 #include "cm_memory.hxx"
 
 #include "cmCommand.h"
-#include "cmFunctionBlocker.h"
-#include "cmListFileCache.h"
 
 class cmExecutionStatus;
-class cmMakefile;
-
-class cmMacroFunctionBlocker : public cmFunctionBlocker
-{
-public:
-  bool IsFunctionBlocked(const cmListFileFunction&, cmMakefile& mf,
-                         cmExecutionStatus&) override;
-  bool ShouldRemove(const cmListFileFunction&, cmMakefile& mf) override;
-
-  std::vector<std::string> Args;
-  std::vector<cmListFileFunction> Functions;
-  int Depth = 0;
-};
 
 /// Starts macro() ... endmacro() block
 class cmMacroCommand : public cmCommand

Source/cmMakefile.cxx

@@ -353,6 +353,11 @@ private:
   cmMakefile* Makefile;
 };
 
+void cmMakefile::OnExecuteCommand(std::function<void()> callback)
+{
+  this->ExecuteCommandCallback = std::move(callback);
+}
+
 bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
                                 cmExecutionStatus& status)
 {
@@ -364,6 +369,10 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
     return result;
   }
 
+  if (this->ExecuteCommandCallback) {
+    this->ExecuteCommandCallback();
+  }
+
   // Place this call on the call stack.
   cmMakefileCall stack_manager(this, lff, status);
   static_cast<void>(stack_manager);
@@ -3061,7 +3070,7 @@ bool cmMakefile::IsFunctionBlocked(const cmListFileFunction& lff,
     return false;
   }
 
-  return this->FunctionBlockers.top()->IsFunctionBlocked(lff, *this, status);
+  return this->FunctionBlockers.top()->IsFunctionBlocked(lff, status);
 }
 
 void cmMakefile::PushFunctionBlockerBarrier()
@@ -3211,30 +3220,12 @@ void cmMakefile::AddFunctionBlocker(std::unique_ptr<cmFunctionBlocker> fb)
   this->FunctionBlockers.push(std::move(fb));
 }
 
-std::unique_ptr<cmFunctionBlocker> cmMakefile::RemoveFunctionBlocker(
-  cmFunctionBlocker* fb, const cmListFileFunction& lff)
+std::unique_ptr<cmFunctionBlocker> cmMakefile::RemoveFunctionBlocker()
 {
   assert(!this->FunctionBlockers.empty());
-  assert(this->FunctionBlockers.top().get() == fb);
   assert(this->FunctionBlockerBarriers.empty() ||
          this->FunctionBlockers.size() > this->FunctionBlockerBarriers.back());
 
-  // Warn if the arguments do not match, but always remove.
-  if (!fb->ShouldRemove(lff, *this)) {
-    cmListFileContext const& lfc = fb->GetStartingContext();
-    cmListFileContext closingContext =
-      cmListFileContext::FromCommandContext(lff, lfc.FilePath);
-    std::ostringstream e;
-    /* clang-format off */
-    e << "A logical block opening on the line\n"
-      << "  " << lfc << "\n"
-      << "closes on the line\n"
-      << "  " << closingContext << "\n"
-      << "with mis-matching arguments.";
-    /* clang-format on */
-    this->IssueMessage(MessageType::AUTHOR_WARNING, e.str());
-  }
-
   auto b = std::move(this->FunctionBlockers.top());
   this->FunctionBlockers.pop();
   return b;

Source/cmMakefile.h

@@ -107,8 +107,7 @@ public:
    * Remove the function blocker whose scope ends with the given command.
    * This returns ownership of the function blocker object.
    */
-  std::unique_ptr<cmFunctionBlocker> RemoveFunctionBlocker(
-    cmFunctionBlocker* fb, const cmListFileFunction& lff);
+  std::unique_ptr<cmFunctionBlocker> RemoveFunctionBlocker();
 
   /**
    * Try running cmake and building a file. This is used for dynalically
@@ -627,6 +626,11 @@ public:
    */
   void PrintCommandTrace(const cmListFileFunction& lff) const;
 
+  /**
+   * Set a callback that is invoked whenever ExecuteCommand is called.
+   */
+  void OnExecuteCommand(std::function<void()> callback);
+
   /**
    * Execute a single CMake command.  Returns true if the command
    * succeeded or false if it failed.
@@ -964,6 +968,7 @@ private:
   bool EnforceUniqueDir(const std::string& srcPath,
                         const std::string& binPath) const;
 
+  std::function<void()> ExecuteCommandCallback;
   using FunctionBlockerPtr = std::unique_ptr<cmFunctionBlocker>;
   using FunctionBlockersType =
     std::stack<FunctionBlockerPtr, std::vector<FunctionBlockerPtr>>;

Source/cmWhileCommand.cxx

@@ -3,10 +3,14 @@
 #include "cmWhileCommand.h"
 
 #include "cm_memory.hxx"
+#include "cm_static_string_view.hxx"
+#include "cm_string_view.hxx"
 
 #include "cmConditionEvaluator.h"
 #include "cmExecutionStatus.h"
 #include "cmExpandedCommandArgument.h"
+#include "cmFunctionBlocker.h"
+#include "cmListFileCache.h"
 #include "cmMakefile.h"
 #include "cmMessageType.h"
 #include "cmSystemTools.h"
@@ -14,9 +18,29 @@
 #include <string>
 #include <utility>
 
+class cmWhileFunctionBlocker : public cmFunctionBlocker
+{
+public:
+  cmWhileFunctionBlocker(cmMakefile* mf);
+  ~cmWhileFunctionBlocker() override;
+
+  cm::string_view StartCommandName() const override { return "while"_s; }
+  cm::string_view EndCommandName() const override { return "endwhile"_s; }
+
+  bool ArgumentsMatch(cmListFileFunction const& lff,
+                      cmMakefile& mf) const override;
+
+  bool Replay(std::vector<cmListFileFunction> functions,
+              cmExecutionStatus& inStatus) override;
+
+  std::vector<cmListFileArgument> Args;
+
+private:
+  cmMakefile* Makefile;
+};
+
 cmWhileFunctionBlocker::cmWhileFunctionBlocker(cmMakefile* mf)
   : Makefile(mf)
-  , Depth(0)
 {
   this->Makefile->PushLoopBlock();
 }
@@ -26,108 +50,77 @@ cmWhileFunctionBlocker::~cmWhileFunctionBlocker()
   this->Makefile->PopLoopBlock();
 }
 
-bool cmWhileFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
-                                               cmMakefile& mf,
-                                               cmExecutionStatus& inStatus)
+bool cmWhileFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
+                                            cmMakefile&) const
 {
-  // at end of for each execute recorded commands
-  if (lff.Name.Lower == "while") {
-    // record the number of while commands past this one
-    this->Depth++;
-  } else if (lff.Name.Lower == "endwhile") {
-    // if this is the endwhile for this while loop then execute
-    if (!this->Depth) {
-      // Remove the function blocker for this scope or bail.
-      std::unique_ptr<cmFunctionBlocker> fb(
-        mf.RemoveFunctionBlocker(this, lff));
-      if (!fb) {
-        return false;
-      }
+  return lff.Arguments.empty() || lff.Arguments == this->Args;
+}
 
-      std::string errorString;
-
-      std::vector<cmExpandedCommandArgument> expandedArguments;
-      mf.ExpandArguments(this->Args, expandedArguments);
-      MessageType messageType;
-
-      cmListFileContext execContext = this->GetStartingContext();
-
-      cmCommandContext commandContext;
-      commandContext.Line = execContext.Line;
-      commandContext.Name = execContext.Name;
-
-      cmConditionEvaluator conditionEvaluator(mf, this->GetStartingContext(),
-                                              mf.GetBacktrace(commandContext));
-
-      bool isTrue =
-        conditionEvaluator.IsTrue(expandedArguments, errorString, messageType);
-
-      while (isTrue) {
-        if (!errorString.empty()) {
-          std::string err = "had incorrect arguments: ";
-          for (cmListFileArgument const& arg : this->Args) {
-            err += (arg.Delim ? "\"" : "");
-            err += arg.Value;
-            err += (arg.Delim ? "\"" : "");
-            err += " ";
-          }
-          err += "(";
-          err += errorString;
-          err += ").";
-          mf.IssueMessage(messageType, err);
-          if (messageType == MessageType::FATAL_ERROR) {
-            cmSystemTools::SetFatalErrorOccured();
-            return true;
-          }
-        }
-
-        // Invoke all the functions that were collected in the block.
-        for (cmListFileFunction const& fn : this->Functions) {
-          cmExecutionStatus status(mf);
-          mf.ExecuteCommand(fn, status);
-          if (status.GetReturnInvoked()) {
-            inStatus.SetReturnInvoked();
-            return true;
-          }
-          if (status.GetBreakInvoked()) {
-            return true;
-          }
-          if (status.GetContinueInvoked()) {
-            break;
-          }
-          if (cmSystemTools::GetFatalErrorOccured()) {
-            return true;
-          }
-        }
-        expandedArguments.clear();
-        mf.ExpandArguments(this->Args, expandedArguments);
-        isTrue = conditionEvaluator.IsTrue(expandedArguments, errorString,
-                                           messageType);
-      }
-      return true;
-    }
-    // decrement for each nested while that ends
-    this->Depth--;
-  }
+bool cmWhileFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
+                                    cmExecutionStatus& inStatus)
+{
+  cmMakefile& mf = inStatus.GetMakefile();
+  std::string errorString;
 
-  // record the command
-  this->Functions.push_back(lff);
+  std::vector<cmExpandedCommandArgument> expandedArguments;
+  mf.ExpandArguments(this->Args, expandedArguments);
+  MessageType messageType;
 
-  // always return true
-  return true;
-}
+  cmListFileContext execContext = this->GetStartingContext();
 
-bool cmWhileFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
-                                          cmMakefile&)
-{
-  if (lff.Name.Lower == "endwhile") {
-    // if the endwhile has arguments, then make sure
-    // they match the arguments of the matching while
-    if (lff.Arguments.empty() || lff.Arguments == this->Args) {
-      return true;
+  cmCommandContext commandContext;
+  commandContext.Line = execContext.Line;
+  commandContext.Name = execContext.Name;
+
+  cmConditionEvaluator conditionEvaluator(mf, this->GetStartingContext(),
+                                          mf.GetBacktrace(commandContext));
+
+  bool isTrue =
+    conditionEvaluator.IsTrue(expandedArguments, errorString, messageType);
+
+  while (isTrue) {
+    if (!errorString.empty()) {
+      std::string err = "had incorrect arguments: ";
+      for (cmListFileArgument const& arg : this->Args) {
+        err += (arg.Delim ? "\"" : "");
+        err += arg.Value;
+        err += (arg.Delim ? "\"" : "");
+        err += " ";
+      }
+      err += "(";
+      err += errorString;
+      err += ").";
+      mf.IssueMessage(messageType, err);
+      if (messageType == MessageType::FATAL_ERROR) {
+        cmSystemTools::SetFatalErrorOccured();
+        return true;
+      }
     }
+
+    // Invoke all the functions that were collected in the block.
+    for (cmListFileFunction const& fn : functions) {
+      cmExecutionStatus status(mf);
+      mf.ExecuteCommand(fn, status);
+      if (status.GetReturnInvoked()) {
+        inStatus.SetReturnInvoked();
+        return true;
+      }
+      if (status.GetBreakInvoked()) {
+        return true;
+      }
+      if (status.GetContinueInvoked()) {
+        break;
+      }
+      if (cmSystemTools::GetFatalErrorOccured()) {
+        return true;
+      }
+    }
+    expandedArguments.clear();
+    mf.ExpandArguments(this->Args, expandedArguments);
+    isTrue =
+      conditionEvaluator.IsTrue(expandedArguments, errorString, messageType);
   }
-  return false;
+  return true;
 }
 
 bool cmWhileCommand(std::vector<cmListFileArgument> const& args,

Source/cmWhileCommand.h

@@ -7,28 +7,8 @@
 
 #include <vector>
 
-#include "cmFunctionBlocker.h"
-#include "cmListFileCache.h"
-
 class cmExecutionStatus;
-class cmMakefile;
-
-class cmWhileFunctionBlocker : public cmFunctionBlocker
-{
-public:
-  cmWhileFunctionBlocker(cmMakefile* mf);
-  ~cmWhileFunctionBlocker() override;
-  bool IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile& mf,
-                         cmExecutionStatus&) override;
-  bool ShouldRemove(const cmListFileFunction& lff, cmMakefile& mf) override;
-
-  std::vector<cmListFileArgument> Args;
-  std::vector<cmListFileFunction> Functions;
-
-private:
-  cmMakefile* Makefile;
-  int Depth;
-};
+struct cmListFileArgument;
 
 /// \brief Starts a while loop
 bool cmWhileCommand(std::vector<cmListFileArgument> const& args,

bootstrap

@@ -326,6 +326,7 @@ CMAKE_CXX_SOURCES="\
   cmFindPathCommand \
   cmFindProgramCommand \
   cmForEachCommand \
+  cmFunctionBlocker \
   cmFunctionCommand \
   cmFSPermissions \
   cmGeneratedFileStream \