[cmake-developers] [PATCH v3 2/2] Reject continue() without loop block (#14013)

Gregor Jasny gjasny at googlemail.com
Sat Nov 15 06:36:33 EST 2014


To track the nested loop levels a counter is used.
It gets incremented while entering a loop block
(e.g. foreach or while) and gets decremented when leaving
the block. Because scope borders for example at function
borders must be taken into account the counter is put into
a stack. With every new scope an empty counter is pushed on
the stack, when leaving the scope the original value is restored.

This enables easy querying if the continue (and later also break)
command is properly nested within a loop scope.

Signed-off-by: Gregor Jasny <gjasny at googlemail.com>
---
 Source/cmContinueCommand.cxx                       |  7 ++++
 Source/cmForEachCommand.cxx                        |  4 ++
 Source/cmMakefile.cxx                              | 47 ++++++++++++++++++++++
 Source/cmMakefile.h                                | 12 ++++++
 Source/cmWhileCommand.cxx                          |  3 ++
 .../RunCMake/continue/NoEnclosingBlock-result.txt  |  1 +
 .../RunCMake/continue/NoEnclosingBlock-stderr.txt  |  2 +
 Tests/RunCMake/continue/NoEnclosingBlock.cmake     |  1 +
 .../continue/NoEnclosingBlockInFunction-result.txt |  1 +
 .../continue/NoEnclosingBlockInFunction-stderr.txt |  2 +
 .../continue/NoEnclosingBlockInFunction.cmake      |  8 ++++
 Tests/RunCMake/continue/RunCMakeTest.cmake         |  2 +
 12 files changed, 90 insertions(+)
 create mode 100644 Tests/RunCMake/continue/NoEnclosingBlock-result.txt
 create mode 100644 Tests/RunCMake/continue/NoEnclosingBlock-stderr.txt
 create mode 100644 Tests/RunCMake/continue/NoEnclosingBlock.cmake
 create mode 100644 Tests/RunCMake/continue/NoEnclosingBlockInFunction-result.txt
 create mode 100644 Tests/RunCMake/continue/NoEnclosingBlockInFunction-stderr.txt
 create mode 100644 Tests/RunCMake/continue/NoEnclosingBlockInFunction.cmake

diff --git a/Source/cmContinueCommand.cxx b/Source/cmContinueCommand.cxx
index d516ad2..1cf2bc7 100644
--- a/Source/cmContinueCommand.cxx
+++ b/Source/cmContinueCommand.cxx
@@ -15,6 +15,13 @@
 bool cmContinueCommand::InitialPass(std::vector<std::string> const&,
                                   cmExecutionStatus &status)
 {
+  if(!this->Makefile->IsLoopBlock())
+    {
+    this->SetError("A CONTINUE command was found outside of a proper "
+                   "FOREACH or WHILE loop scope.");
+    return false;
+    }
+
   status.SetContinueInvoked(true);
   return true;
 }
diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx
index 465ddab..4e16430 100644
--- a/Source/cmForEachCommand.cxx
+++ b/Source/cmForEachCommand.cxx
@@ -77,6 +77,8 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
             }
           }
         }
+      mf.PopLoopBlock();
+
       // restore the variable to its prior value
       mf.AddDefinition(this->Args[0],oldDef.c_str());
       return true;
@@ -203,6 +205,8 @@ bool cmForEachCommand
     }
   this->Makefile->AddFunctionBlocker(f);
 
+  this->Makefile->PushLoopBlock();
+
   return true;
 }
 
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 8a8aadc..4f652af 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -171,6 +171,9 @@ void cmMakefile::Initialize()
   // Protect the directory-level policies.
   this->PushPolicyBarrier();
 
+  // push empty loop block
+  this->PushLoopBlockBarrier();
+
   // By default the check is not done.  It is enabled by
   // cmListFileCache in the top level if necessary.
   this->CheckCMP0000 = false;
@@ -3293,6 +3296,46 @@ void cmMakefile::PopFunctionBlockerBarrier(bool reportError)
 }
 
 //----------------------------------------------------------------------------
+void cmMakefile::PushLoopBlock()
+{
+  if(this->LoopBlockCounter.empty())
+    {
+    assert(false);
+    return;
+    }
+
+  this->LoopBlockCounter.top()++;
+}
+
+void cmMakefile::PopLoopBlock()
+{
+  if(this->LoopBlockCounter.empty() || this->LoopBlockCounter.top() <= 0)
+    {
+    assert(false);
+    return;
+    }
+
+  this->LoopBlockCounter.top()--;
+}
+
+void cmMakefile::PushLoopBlockBarrier()
+{
+  this->LoopBlockCounter.push(0);
+}
+
+void cmMakefile::PopLoopBlockBarrier()
+{
+  assert(!this->LoopBlockCounter.empty() && this->LoopBlockCounter.top() == 0);
+  this->LoopBlockCounter.pop();
+}
+
+bool cmMakefile::IsLoopBlock() const
+{
+  assert(!this->LoopBlockCounter.empty());
+  return !this->LoopBlockCounter.empty() && this->LoopBlockCounter.top() > 0;
+}
+
+//----------------------------------------------------------------------------
 bool cmMakefile::ExpandArguments(
   std::vector<cmListFileArgument> const& inArgs,
   std::vector<std::string>& outArgs) const
@@ -4426,10 +4469,14 @@ void cmMakefile::PushScope()
   this->Internal->VarStack.push(cmDefinitions(parent));
   this->Internal->VarInitStack.push(init);
   this->Internal->VarUsageStack.push(usage);
+
+  PushLoopBlockBarrier();
 }
 
 void cmMakefile::PopScope()
 {
+  PopLoopBlockBarrier();
+
   cmDefinitions* current = &this->Internal->VarStack.top();
   std::set<std::string> init = this->Internal->VarInitStack.top();
   std::set<std::string> usage = this->Internal->VarUsageStack.top();
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 824513b..7c746fe 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -34,6 +34,8 @@
 # include <cmsys/hash_map.hxx>
 #endif
 
+#include <stack>
+
 class cmFunctionBlocker;
 class cmCommand;
 class cmInstallGenerator;
@@ -885,6 +887,10 @@ public:
   void PopScope();
   void RaiseScope(const std::string& var, const char *value);
 
+  // push and pop loop scopes
+  void PushLoopBlockBarrier();
+  void PopLoopBlockBarrier();
+
   /** Helper class to push and pop scopes automatically.  */
   class ScopePushPop
   {
@@ -945,6 +951,10 @@ public:
   void ClearMatches();
   void StoreMatches(cmsys::RegularExpression& re);
 
+  void PushLoopBlock();
+  void PopLoopBlock();
+  bool IsLoopBlock() const;
+
 protected:
   // add link libraries and directories to the target
   void AddGlobalLinkInformation(const std::string& name, cmTarget& target);
@@ -1039,6 +1049,8 @@ private:
   void PushFunctionBlockerBarrier();
   void PopFunctionBlockerBarrier(bool reportError = true);
 
+  std::stack<int> LoopBlockCounter;
+
   typedef std::map<std::string, std::string> StringStringMap;
   StringStringMap MacrosMap;
 
diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx
index 98e129d..b8fbffe 100644
--- a/Source/cmWhileCommand.cxx
+++ b/Source/cmWhileCommand.cxx
@@ -95,6 +95,7 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
         isTrue = conditionEvaluator.IsTrue(
           expandedArguments, errorString, messageType);
         }
+      mf.PopLoopBlock();
       return true;
       }
     else
@@ -142,6 +143,8 @@ bool cmWhileCommand
   f->Args = args;
   this->Makefile->AddFunctionBlocker(f);
 
+  this->Makefile->PushLoopBlock();
+
   return true;
 }
 
diff --git a/Tests/RunCMake/continue/NoEnclosingBlock-result.txt b/Tests/RunCMake/continue/NoEnclosingBlock-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/continue/NoEnclosingBlock-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/continue/NoEnclosingBlock-stderr.txt b/Tests/RunCMake/continue/NoEnclosingBlock-stderr.txt
new file mode 100644
index 0000000..b3b3f96
--- /dev/null
+++ b/Tests/RunCMake/continue/NoEnclosingBlock-stderr.txt
@@ -0,0 +1,2 @@
+  continue A CONTINUE command was found outside of a proper FOREACH or WHILE
+  loop scope.
diff --git a/Tests/RunCMake/continue/NoEnclosingBlock.cmake b/Tests/RunCMake/continue/NoEnclosingBlock.cmake
new file mode 100644
index 0000000..9661e0d
--- /dev/null
+++ b/Tests/RunCMake/continue/NoEnclosingBlock.cmake
@@ -0,0 +1 @@
+continue()
diff --git a/Tests/RunCMake/continue/NoEnclosingBlockInFunction-result.txt b/Tests/RunCMake/continue/NoEnclosingBlockInFunction-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/continue/NoEnclosingBlockInFunction-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/continue/NoEnclosingBlockInFunction-stderr.txt b/Tests/RunCMake/continue/NoEnclosingBlockInFunction-stderr.txt
new file mode 100644
index 0000000..b3b3f96
--- /dev/null
+++ b/Tests/RunCMake/continue/NoEnclosingBlockInFunction-stderr.txt
@@ -0,0 +1,2 @@
+  continue A CONTINUE command was found outside of a proper FOREACH or WHILE
+  loop scope.
diff --git a/Tests/RunCMake/continue/NoEnclosingBlockInFunction.cmake b/Tests/RunCMake/continue/NoEnclosingBlockInFunction.cmake
new file mode 100644
index 0000000..eb2a098
--- /dev/null
+++ b/Tests/RunCMake/continue/NoEnclosingBlockInFunction.cmake
@@ -0,0 +1,8 @@
+function(foo)
+  continue()
+endfunction(foo)
+
+foreach(i RANGE 1 2)
+  foo()
+  message(STATUS "Hello World")
+endforeach()
diff --git a/Tests/RunCMake/continue/RunCMakeTest.cmake b/Tests/RunCMake/continue/RunCMakeTest.cmake
index c057282..f8fef6a 100644
--- a/Tests/RunCMake/continue/RunCMakeTest.cmake
+++ b/Tests/RunCMake/continue/RunCMakeTest.cmake
@@ -3,3 +3,5 @@ include(RunCMake)
 run_cmake(ContinueForeach)
 run_cmake(ContinueNestedForeach)
 run_cmake(ContinueWhile)
+run_cmake(NoEnclosingBlock)
+run_cmake(NoEnclosingBlockInFunction)
-- 
1.9.3 (Apple Git-50)



More information about the cmake-developers mailing list