From 2c81e5fb5cd592e9450250364e6667082014f0b7 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 20 Jan 2009 14:36:18 -0500 Subject: 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 | 15 ++++----------- Source/cmForEachCommand.h | 3 +-- Source/cmIfCommand.cxx | 16 ++++------------ Source/cmIfCommand.h | 3 +-- Source/cmMakefile.cxx | 8 ++++---- Source/cmMakefile.h | 11 ++++++++--- Source/cmWhileCommand.cxx | 15 ++++----------- Source/cmWhileCommand.h | 3 +-- 8 files changed, 27 insertions(+), 47 deletions(-) diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx index b13a849..193f0a0 100644 --- a/Source/cmForEachCommand.cxx +++ b/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 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::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 diff --git a/Source/cmForEachCommand.h b/Source/cmForEachCommand.h index 6ef217a..3357ce4 100644 --- a/Source/cmForEachCommand.h +++ b/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 Args; std::vector Functions; - bool Executing; private: int Depth; }; diff --git a/Source/cmIfCommand.cxx b/Source/cmIfCommand.cxx index cbba2d3..1a8c810 100644 --- a/Source/cmIfCommand.cxx +++ b/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 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; } } diff --git a/Source/cmIfCommand.h b/Source/cmIfCommand.h index bc84e56..0d2802a 100644 --- a/Source/cmIfCommand.h +++ b/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 diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index bc013f9..ed70c07 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2395,7 +2395,8 @@ bool cmMakefile::ExpandArguments( return !cmSystemTools::GetFatalErrorOccured(); } -void cmMakefile::RemoveFunctionBlocker(const cmListFileFunction& lff) +cmsys::auto_ptr +cmMakefile::RemoveFunctionBlocker(const cmListFileFunction& lff) { // loop over all function blockers to see if any block this command std::list::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(b); } } - return; + return cmsys::auto_ptr(); } void cmMakefile::SetHomeDirectory(const char* dir) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 6a09042..6da3646 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -31,6 +31,7 @@ #include "cmSourceGroup.h" #endif +#include #include 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 + RemoveFunctionBlocker(const cmListFileFunction& lff); /** * Try running cmake and building a file. This is used for dynalically diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx index 21fc286..61d5d85 100644 --- a/Source/cmWhileCommand.cxx +++ b/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 fb(mf.RemoveFunctionBlocker(lff)); + if(!fb.get()) { return false; } + std::string errorString; std::vector 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 diff --git a/Source/cmWhileCommand.h b/Source/cmWhileCommand.h index c95df73..39864cd 100644 --- a/Source/cmWhileCommand.h +++ b/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 Args; std::vector Functions; - bool Executing; private: int Depth; }; -- cgit v0.12