From 1dcc5b455829db89f0d931cd57c12e0e247c0f44 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 21 Jan 2009 09:49:00 -0500 Subject: [PATCH] ENH: Better handling of mismatched blocks If a logical block terminates with mismatching arguments we previously failed to remove the function blocker but replayed the commands anyway, which led to cases in which we failed to report the mismatch (return shortly after the ending command). The recent refactoring of function blocker deletion changed this behavior to produce an error on the ending line by not blocking the command. Furthermore, the function blocker would stay in place and complain at the end of every equal-level block of the same type. This teaches CMake to treat the begin/end commands (if/endif, etc.) as correct and just warns when the arguments mismatch. The change allows cases in which CMake 2.6.2 silently ignored a mismatch to run as before but with a warning. --- Source/cmForEachCommand.cxx | 3 ++- Source/cmFunctionCommand.cxx | 2 +- Source/cmIfCommand.cxx | 3 ++- Source/cmMacroCommand.cxx | 2 +- Source/cmMakefile.cxx | 17 +++++++++++++++-- Source/cmMakefile.h | 2 +- Source/cmWhileCommand.cxx | 3 ++- 7 files changed, 24 insertions(+), 8 deletions(-) diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx index 436a91bcb..7a826c631 100644 --- a/Source/cmForEachCommand.cxx +++ b/Source/cmForEachCommand.cxx @@ -31,7 +31,8 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, if (!this->Depth) { // Remove the function blocker for this scope or bail. - cmsys::auto_ptr fb(mf.RemoveFunctionBlocker(lff)); + cmsys::auto_ptr + fb(mf.RemoveFunctionBlocker(this, lff)); if(!fb.get()) { return false; } // at end of for each execute recorded commands diff --git a/Source/cmFunctionCommand.cxx b/Source/cmFunctionCommand.cxx index c1ca8aa29..04d842888 100644 --- a/Source/cmFunctionCommand.cxx +++ b/Source/cmFunctionCommand.cxx @@ -222,7 +222,7 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, mf.AddCommand(f); // remove the function blocker now that the function is defined - mf.RemoveFunctionBlocker(lff); + mf.RemoveFunctionBlocker(this, lff); return true; } else diff --git a/Source/cmIfCommand.cxx b/Source/cmIfCommand.cxx index 74576b5ae..607031e57 100644 --- a/Source/cmIfCommand.cxx +++ b/Source/cmIfCommand.cxx @@ -39,7 +39,8 @@ IsFunctionBlocked(const cmListFileFunction& lff, if (!this->ScopeDepth) { // Remove the function blocker for this scope or bail. - cmsys::auto_ptr fb(mf.RemoveFunctionBlocker(lff)); + cmsys::auto_ptr + fb(mf.RemoveFunctionBlocker(this, lff)); if(!fb.get()) { return false; } // execute the functions for the true parts of the if statement diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 8613044a5..0a7d0dc7b 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -266,7 +266,7 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, mf.AddCommand(f); // remove the function blocker now that the macro is defined - mf.RemoveFunctionBlocker(lff); + mf.RemoveFunctionBlocker(this, lff); return true; } else diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index c6bc4f217..890f75290 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2422,7 +2422,8 @@ void cmMakefile::AddFunctionBlocker(cmFunctionBlocker* fb) } cmsys::auto_ptr -cmMakefile::RemoveFunctionBlocker(const cmListFileFunction& lff) +cmMakefile::RemoveFunctionBlocker(cmFunctionBlocker* fb, + const cmListFileFunction& lff) { // Find the function blocker stack barrier for the current scope. // We only remove a blocker whose index is not less than the barrier. @@ -2438,8 +2439,20 @@ cmMakefile::RemoveFunctionBlocker(const cmListFileFunction& lff) { std::vector::iterator pos = this->FunctionBlockers.begin() + (i - 1); - if ((*pos)->ShouldRemove(lff, *this)) + if (*pos == fb) { + // Warn if the arguments do not match, but always remove. + if(!(*pos)->ShouldRemove(lff, *this)) + { + cmListFileContext const& lfc = fb->GetStartingContext(); + cmOStringStream e; + e << "A logical block opening on the line\n" + << " " << lfc << "\n" + << "closes on the line\n" + << " " << lff << "\n" + << "with mis-matching arguments."; + this->IssueMessage(cmake::AUTHOR_WARNING, e.str()); + } cmFunctionBlocker* b = *pos; this->FunctionBlockers.erase(pos); return cmsys::auto_ptr(b); diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index d565b2907..c6061d73c 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -95,7 +95,7 @@ public: * This returns ownership of the function blocker object. */ cmsys::auto_ptr - RemoveFunctionBlocker(const cmListFileFunction& lff); + RemoveFunctionBlocker(cmFunctionBlocker* fb, const cmListFileFunction& lff); /** Push/pop a lexical (function blocker) barrier automatically. */ class LexicalPushPop diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx index b6d8e36e7..cc1651207 100644 --- a/Source/cmWhileCommand.cxx +++ b/Source/cmWhileCommand.cxx @@ -33,7 +33,8 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, if (!this->Depth) { // Remove the function blocker for this scope or bail. - cmsys::auto_ptr fb(mf.RemoveFunctionBlocker(lff)); + cmsys::auto_ptr + fb(mf.RemoveFunctionBlocker(this, lff)); if(!fb.get()) { return false; } std::string errorString;