ENH: Refactor function blocker deletion

When a function blocker decides to remove itself we previously removed
it at every return point from the C++ scope in which its removal is
needed.  This teaches function blockers to transfer ownership of
themselves from cmMakefile to an automatic variable for deletion on
return.  Since this removes blockers before they replay their commands,
we no longer need to avoid running blockers on their own commands.

Source/cmForEachCommand.cxx

@@ -20,13 +20,6 @@ bool cmForEachFunctionBlocker::
 IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
                   cmExecutionStatus &inStatus)
 {
-  // Prevent recusion and don't let this blocker block its own
-  // commands.
-  if (this->Executing)
-    {
-    return false;
-    }
-
   if (!cmSystemTools::Strucmp(lff.Name.c_str(),"foreach"))
     {
     // record the number of nested foreach commands
@@ -37,6 +30,10 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
     // if this is the endofreach for this statement
     if (!this->Depth) 
       {
+      // Remove the function blocker for this scope or bail.
+      cmsys::auto_ptr<cmFunctionBlocker> fb(mf.RemoveFunctionBlocker(lff));
+      if(!fb.get()) { return false; }
+
       // at end of for each execute recorded commands
       // store the old value
       std::string oldDef;
@@ -44,7 +41,6 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
         {
         oldDef = mf.GetDefinition(this->Args[0].c_str());
         }
-      this->Executing = true;
       std::vector<std::string>::const_iterator j = this->Args.begin();
       ++j;
 
@@ -65,21 +61,18 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
             inStatus.SetReturnInvoked(true);
             // restore the variable to its prior value
             mf.AddDefinition(this->Args[0].c_str(),oldDef.c_str());
-            mf.RemoveFunctionBlocker(lff);
             return true;
             }
           if (status.GetBreakInvoked())
             {
             // restore the variable to its prior value
             mf.AddDefinition(this->Args[0].c_str(),oldDef.c_str());
-            mf.RemoveFunctionBlocker(lff);
             return true;
             }
           }
         }
       // restore the variable to its prior value
       mf.AddDefinition(this->Args[0].c_str(),oldDef.c_str());
-      mf.RemoveFunctionBlocker(lff);
       return true;
       }
     else

Source/cmForEachCommand.h

@@ -29,7 +29,7 @@
 class cmForEachFunctionBlocker : public cmFunctionBlocker
 {
 public:
-  cmForEachFunctionBlocker() {this->Executing = false; Depth = 0;}
+  cmForEachFunctionBlocker() {this->Depth = 0;}
   virtual ~cmForEachFunctionBlocker() {}
   virtual bool IsFunctionBlocked(const cmListFileFunction& lff,
                                  cmMakefile &mf,
@@ -39,7 +39,6 @@ public:
   
   std::vector<std::string> Args;
   std::vector<cmListFileFunction> Functions;
-  bool Executing;
 private:
   int Depth;
 };

Source/cmIfCommand.cxx

@@ -27,13 +27,6 @@ IsFunctionBlocked(const cmListFileFunction& lff,
                   cmMakefile &mf,
                   cmExecutionStatus &inStatus)
 {
-  // Prevent recusion and don't let this blocker block its own
-  // commands.
-  if (this->Executing)
-    {
-    return false;
-    }
-
   // we start by recording all the functions
   if (!cmSystemTools::Strucmp(lff.Name.c_str(),"if"))
     {
@@ -45,8 +38,11 @@ IsFunctionBlocked(const cmListFileFunction& lff,
     // if this is the endif for this if statement, then start executing
     if (!this->ScopeDepth) 
       {
+      // Remove the function blocker for this scope or bail.
+      cmsys::auto_ptr<cmFunctionBlocker> fb(mf.RemoveFunctionBlocker(lff));
+      if(!fb.get()) { return false; }
+
       // execute the functions for the true parts of the if statement
-      this->Executing = true;
       cmExecutionStatus status;
       int scopeDepth = 0;
       for(unsigned int c = 0; c < this->Functions.size(); ++c)
@@ -104,7 +100,6 @@ IsFunctionBlocked(const cmListFileFunction& lff,
               err += ").";
               mf.IssueMessage(cmake::FATAL_ERROR, err);
               cmSystemTools::SetFatalErrorOccured();
-              mf.RemoveFunctionBlocker(lff);
               return true;
               }
         
@@ -124,18 +119,15 @@ IsFunctionBlocked(const cmListFileFunction& lff,
           if (status.GetReturnInvoked())
             {
             inStatus.SetReturnInvoked(true);
-            mf.RemoveFunctionBlocker(lff);
             return true;
             }
           if (status.GetBreakInvoked())
             {
             inStatus.SetBreakInvoked(true);
-            mf.RemoveFunctionBlocker(lff);
             return true;
             }
           }
         }
-      mf.RemoveFunctionBlocker(lff);
       return true;
       }
     }

Source/cmIfCommand.h

@@ -29,7 +29,7 @@ class cmIfFunctionBlocker : public cmFunctionBlocker
 {
 public:
   cmIfFunctionBlocker() {
-    this->HasRun = false; this->ScopeDepth = 0; this->Executing = false;}
+    this->HasRun = false; this->ScopeDepth = 0; }
   virtual ~cmIfFunctionBlocker() {}
   virtual bool IsFunctionBlocked(const cmListFileFunction& lff,
                                  cmMakefile &mf,
@@ -43,7 +43,6 @@ public:
   bool IsBlocking;
   bool HasRun;
   unsigned int ScopeDepth;
-  bool Executing;
 };
 
 /** \class cmIfCommand

Source/cmMakefile.cxx

@@ -2395,7 +2395,8 @@ bool cmMakefile::ExpandArguments(
   return !cmSystemTools::GetFatalErrorOccured();
 }
 
-void cmMakefile::RemoveFunctionBlocker(const cmListFileFunction& lff)
+cmsys::auto_ptr<cmFunctionBlocker>
+cmMakefile::RemoveFunctionBlocker(const cmListFileFunction& lff)
 {
   // loop over all function blockers to see if any block this command
   std::list<cmFunctionBlocker *>::reverse_iterator pos;
@@ -2406,12 +2407,11 @@ void cmMakefile::RemoveFunctionBlocker(const cmListFileFunction& lff)
       {
       cmFunctionBlocker* b = *pos;
       this->FunctionBlockers.remove(b);
-      delete b;
-      break;
+      return cmsys::auto_ptr<cmFunctionBlocker>(b);
       }
     }
 
-  return;
+  return cmsys::auto_ptr<cmFunctionBlocker>();
 }
 
 void cmMakefile::SetHomeDirectory(const char* dir)

Source/cmMakefile.h

@@ -31,6 +31,7 @@
 #include "cmSourceGroup.h"
 #endif
 
+#include <cmsys/auto_ptr.hxx>
 #include <cmsys/RegularExpression.hxx>
 
 class cmFunctionBlocker;
@@ -89,9 +90,13 @@ public:
    */
   void AddFunctionBlocker(cmFunctionBlocker *fb)
     { this->FunctionBlockers.push_back(fb);}
-  void RemoveFunctionBlocker(cmFunctionBlocker *fb)
-    { this->FunctionBlockers.remove(fb);}
-  void RemoveFunctionBlocker(const cmListFileFunction& lff);
+
+  /**
+   * Remove the function blocker whose scope ends with the given command.
+   * This returns ownership of the function blocker object.
+   */
+  cmsys::auto_ptr<cmFunctionBlocker>
+  RemoveFunctionBlocker(const cmListFileFunction& lff);
 
   /**
    * Try running cmake and building a file. This is used for dynalically

Source/cmWhileCommand.cxx

@@ -21,13 +21,6 @@ bool cmWhileFunctionBlocker::
 IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
                   cmExecutionStatus &inStatus)
 {
-  // Prevent recusion and don't let this blocker block its own
-  // commands.
-  if (this->Executing)
-    {
-    return false;
-    }
-  
   // at end of for each execute recorded commands
   if (!cmSystemTools::Strucmp(lff.Name.c_str(),"while"))
     {
@@ -39,6 +32,10 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
     // if this is the endwhile for this while loop then execute
     if (!this->Depth) 
       {
+      // Remove the function blocker for this scope or bail.
+      cmsys::auto_ptr<cmFunctionBlocker> fb(mf.RemoveFunctionBlocker(lff));
+      if(!fb.get()) { return false; }
+
       std::string errorString;
     
       std::vector<std::string> expandedArguments;
@@ -46,7 +43,6 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
       bool isTrue = 
         cmIfCommand::IsTrue(expandedArguments,errorString,&mf);
 
-      this->Executing = true;
       while (isTrue)
         {      
         // Invoke all the functions that were collected in the block.
@@ -57,12 +53,10 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
           if (status.GetReturnInvoked())
             {
             inStatus.SetReturnInvoked(true);
-            mf.RemoveFunctionBlocker(lff);
             return true;
             }
           if (status.GetBreakInvoked())
             {
-            mf.RemoveFunctionBlocker(lff);
             return true;
             }
           }
@@ -71,7 +65,6 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
         isTrue = 
           cmIfCommand::IsTrue(expandedArguments,errorString,&mf);
         }
-      mf.RemoveFunctionBlocker(lff);
       return true;
       }
     else

Source/cmWhileCommand.h

@@ -29,7 +29,7 @@
 class cmWhileFunctionBlocker : public cmFunctionBlocker
 {
 public:
-  cmWhileFunctionBlocker() {Executing = false; Depth=0;}
+  cmWhileFunctionBlocker() {this->Depth=0;}
   virtual ~cmWhileFunctionBlocker() {}
   virtual bool IsFunctionBlocked(const cmListFileFunction& lff,
                                  cmMakefile &mf,
@@ -39,7 +39,6 @@ public:
   
   std::vector<cmListFileArgument> Args;
   std::vector<cmListFileFunction> Functions;
-  bool Executing;
 private:
   int Depth;
 };