[cmake-developers] [PATCH v4 2/3] Reject continue() without loop block (#14013)
Gregor Jasny
gjasny at googlemail.com
Mon Nov 17 14:00:53 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 | 8 ++++
Source/cmMakefile.cxx | 48 ++++++++++++++++++++++
Source/cmMakefile.h | 21 ++++++++++
Source/cmWhileCommand.cxx | 4 ++
Tests/RunCMake/CMakeLists.txt | 1 +
.../RunCMake/continue/ContinueForEachInLists.cmake | 10 +++++
.../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 | 3 ++
Tests/RunCMake/return/CMakeLists.txt | 3 ++
Tests/RunCMake/return/ReturnFromForeach-result.txt | 1 +
Tests/RunCMake/return/ReturnFromForeach.cmake | 10 +++++
Tests/RunCMake/return/RunCMakeTest.cmake | 3 ++
18 files changed, 134 insertions(+)
create mode 100644 Tests/RunCMake/continue/ContinueForEachInLists.cmake
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
create mode 100644 Tests/RunCMake/return/CMakeLists.txt
create mode 100644 Tests/RunCMake/return/ReturnFromForeach-result.txt
create mode 100644 Tests/RunCMake/return/ReturnFromForeach.cmake
create mode 100644 Tests/RunCMake/return/RunCMakeTest.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..03d6590 100644
--- a/Source/cmForEachCommand.cxx
+++ b/Source/cmForEachCommand.cxx
@@ -27,6 +27,8 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
// if this is the endofreach for this statement
if (!this->Depth)
{
+ cmMakefile::LoopBlockPop loopBlockPop(&mf);
+
// Remove the function blocker for this scope or bail.
cmsys::auto_ptr<cmFunctionBlocker>
fb(mf.RemoveFunctionBlocker(this, lff));
@@ -77,6 +79,7 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
}
}
}
+
// restore the variable to its prior value
mf.AddDefinition(this->Args[0],oldDef.c_str());
return true;
@@ -203,6 +206,8 @@ bool cmForEachCommand
}
this->Makefile->AddFunctionBlocker(f);
+ this->Makefile->PushLoopBlock();
+
return true;
}
@@ -246,5 +251,8 @@ bool cmForEachCommand::HandleInMode(std::vector<std::string> const& args)
}
this->Makefile->AddFunctionBlocker(f.release()); // TODO: pass auto_ptr
+
+ this->Makefile->PushLoopBlock();
+
return true;
}
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 8a8aadc..2795e83 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,47 @@ 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());
+ assert(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 +4470,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..809c77d 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;
@@ -123,6 +125,15 @@ public:
};
friend class LexicalPushPop;
+ class LoopBlockPop
+ {
+ public:
+ LoopBlockPop(cmMakefile* mf) { this->Makefile = mf; }
+ ~LoopBlockPop() { this->Makefile->PopLoopBlock(); }
+ private:
+ cmMakefile* Makefile;
+ };
+
/**
* Try running cmake and building a file. This is used for dynalically
* loaded commands, not as part of the usual build process.
@@ -885,6 +896,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 +960,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 +1058,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..47edb03 100644
--- a/Source/cmWhileCommand.cxx
+++ b/Source/cmWhileCommand.cxx
@@ -27,6 +27,8 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
// if this is the endwhile for this while loop then execute
if (!this->Depth)
{
+ cmMakefile::LoopBlockPop loopBlockPop(&mf);
+
// Remove the function blocker for this scope or bail.
cmsys::auto_ptr<cmFunctionBlocker>
fb(mf.RemoveFunctionBlocker(this, lff));
@@ -142,6 +144,8 @@ bool cmWhileCommand
f->Args = args;
this->Makefile->AddFunctionBlocker(f);
+ this->Makefile->PushLoopBlock();
+
return true;
}
diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt
index 0099d4b..24d0c93 100644
--- a/Tests/RunCMake/CMakeLists.txt
+++ b/Tests/RunCMake/CMakeLists.txt
@@ -109,6 +109,7 @@ add_RunCMake_test(include_directories)
add_RunCMake_test(list)
add_RunCMake_test(message)
add_RunCMake_test(project)
+add_RunCMake_test(return)
add_RunCMake_test(string)
add_RunCMake_test(try_compile)
add_RunCMake_test(set)
diff --git a/Tests/RunCMake/continue/ContinueForEachInLists.cmake b/Tests/RunCMake/continue/ContinueForEachInLists.cmake
new file mode 100644
index 0000000..fbd7359
--- /dev/null
+++ b/Tests/RunCMake/continue/ContinueForEachInLists.cmake
@@ -0,0 +1,10 @@
+list(APPEND foo 1 2 3 4 5)
+
+message(STATUS "start")
+foreach(iter IN LISTS foo)
+ if("${iter}" EQUAL 1 OR "${iter}" EQUAL 3 OR "${iter}" EQUAL 5)
+ continue()
+ endif()
+ message(STATUS "${iter}")
+endforeach()
+message(STATUS "end")
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..f85154f 100644
--- a/Tests/RunCMake/continue/RunCMakeTest.cmake
+++ b/Tests/RunCMake/continue/RunCMakeTest.cmake
@@ -1,5 +1,8 @@
include(RunCMake)
run_cmake(ContinueForeach)
+run_cmake(ContinueForEachInLists)
run_cmake(ContinueNestedForeach)
run_cmake(ContinueWhile)
+run_cmake(NoEnclosingBlock)
+run_cmake(NoEnclosingBlockInFunction)
diff --git a/Tests/RunCMake/return/CMakeLists.txt b/Tests/RunCMake/return/CMakeLists.txt
new file mode 100644
index 0000000..2d75985
--- /dev/null
+++ b/Tests/RunCMake/return/CMakeLists.txt
@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 3.1)
+project(${RunCMake_TEST} CXX)
+include(${RunCMake_TEST}.cmake)
diff --git a/Tests/RunCMake/return/ReturnFromForeach-result.txt b/Tests/RunCMake/return/ReturnFromForeach-result.txt
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/Tests/RunCMake/return/ReturnFromForeach-result.txt
@@ -0,0 +1 @@
+0
diff --git a/Tests/RunCMake/return/ReturnFromForeach.cmake b/Tests/RunCMake/return/ReturnFromForeach.cmake
new file mode 100644
index 0000000..c71cf33
--- /dev/null
+++ b/Tests/RunCMake/return/ReturnFromForeach.cmake
@@ -0,0 +1,10 @@
+function(foo)
+ foreach(i RANGE 1 3)
+ foreach(j RANGE 1 3)
+ return()
+ message(FATAL_ERROR "unexpected")
+ endforeach()
+ endforeach()
+endfunction(foo)
+
+foo()
diff --git a/Tests/RunCMake/return/RunCMakeTest.cmake b/Tests/RunCMake/return/RunCMakeTest.cmake
new file mode 100644
index 0000000..2cc6c9d
--- /dev/null
+++ b/Tests/RunCMake/return/RunCMakeTest.cmake
@@ -0,0 +1,3 @@
+include(RunCMake)
+
+run_cmake(ReturnFromForeach)
--
1.9.3 (Apple Git-50)
More information about the cmake-developers
mailing list