diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst index 7074bd5dc..742fd6362 100644 --- a/Help/manual/cmake-policies.7.rst +++ b/Help/manual/cmake-policies.7.rst @@ -112,3 +112,4 @@ All Policies /policy/CMP0052 /policy/CMP0053 /policy/CMP0054 + /policy/CMP0055 diff --git a/Help/policy/CMP0055.rst b/Help/policy/CMP0055.rst new file mode 100644 index 000000000..fe7ab6f6d --- /dev/null +++ b/Help/policy/CMP0055.rst @@ -0,0 +1,17 @@ +CMP0055 +------- + +Strict checking for the :command:`break` command. + +CMake 3.1 and lower allowed calls to the :command:`break` command +outside of a loop context and also ignored any given arguments. +This was undefined behavior. + +The OLD behavior for this policy is to allow :command:`break` to be placed +outside of loop contexts and ignores any arguments. The NEW behavior for this +policy is to issue an error if a misplaced break or any arguments are found. + +This policy was introduced in CMake version 3.2. +CMake version |release| warns when the policy is not set and uses +OLD behavior. Use the cmake_policy command to set it to OLD or +NEW explicitly. diff --git a/Help/release/dev/break-command-strictness.rst b/Help/release/dev/break-command-strictness.rst new file mode 100644 index 000000000..0723774ed --- /dev/null +++ b/Help/release/dev/break-command-strictness.rst @@ -0,0 +1,6 @@ +break-command-strictness +------------------------ + +* The :command:`break` command now rejects calls outside of a loop + context or that pass arguments to the command. + See policy :policy:`CMP0055`. diff --git a/Source/cmBreakCommand.cxx b/Source/cmBreakCommand.cxx index b70e40088..ff527db54 100644 --- a/Source/cmBreakCommand.cxx +++ b/Source/cmBreakCommand.cxx @@ -12,10 +12,76 @@ #include "cmBreakCommand.h" // cmBreakCommand -bool cmBreakCommand::InitialPass(std::vector const&, +bool cmBreakCommand::InitialPass(std::vector const &args, cmExecutionStatus &status) { + if(!this->Makefile->IsLoopBlock()) + { + bool issueMessage = true; + cmOStringStream e; + cmake::MessageType messageType = cmake::AUTHOR_WARNING; + switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0055)) + { + case cmPolicies::WARN: + e << (this->Makefile->GetPolicies() + ->GetPolicyWarning(cmPolicies::CMP0055)) << "\n"; + break; + case cmPolicies::OLD: + issueMessage = false; + break; + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::NEW: + messageType = cmake::FATAL_ERROR; + break; + } + + if(issueMessage) + { + e << "A BREAK command was found outside of a proper " + "FOREACH or WHILE loop scope."; + this->Makefile->IssueMessage(messageType, e.str()); + if(messageType == cmake::FATAL_ERROR) + { + return false; + } + } + } + status.SetBreakInvoked(true); + + if(!args.empty()) + { + bool issueMessage = true; + cmOStringStream e; + cmake::MessageType messageType = cmake::AUTHOR_WARNING; + switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0055)) + { + case cmPolicies::WARN: + e << (this->Makefile->GetPolicies() + ->GetPolicyWarning(cmPolicies::CMP0055)) << "\n"; + break; + case cmPolicies::OLD: + issueMessage = false; + break; + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::NEW: + messageType = cmake::FATAL_ERROR; + break; + } + + if(issueMessage) + { + e << "The BREAK command does not accept any arguments."; + this->Makefile->IssueMessage(messageType, e.str()); + if(messageType == cmake::FATAL_ERROR) + { + return false; + } + } + } + return true; } diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx index e3f66c1c3..dec51579a 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 fb(mf.RemoveFunctionBlocker(this, lff)); @@ -73,6 +75,7 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, } } } + // restore the variable to its prior value mf.AddDefinition(this->Args[0],oldDef.c_str()); return true; @@ -199,6 +202,8 @@ bool cmForEachCommand } this->Makefile->AddFunctionBlocker(f); + this->Makefile->PushLoopBlock(); + return true; } @@ -242,5 +247,8 @@ bool cmForEachCommand::HandleInMode(std::vector 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 61807b2d5..6eca0f0bf 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; @@ -3353,6 +3356,38 @@ void cmMakefile::PopFunctionBlockerBarrier(bool reportError) this->FunctionBlockerBarriers.pop_back(); } +//---------------------------------------------------------------------------- +void cmMakefile::PushLoopBlock() +{ + assert(!this->LoopBlockCounter.empty()); + this->LoopBlockCounter.top()++; +} + +void cmMakefile::PopLoopBlock() +{ + assert(!this->LoopBlockCounter.empty()); + assert(this->LoopBlockCounter.top() > 0); + 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 const& inArgs, @@ -4487,10 +4522,14 @@ void cmMakefile::PushScope() this->Internal->VarStack.push(cmDefinitions(parent)); this->Internal->VarInitStack.push(init); this->Internal->VarUsageStack.push(usage); + + this->PushLoopBlockBarrier(); } void cmMakefile::PopScope() { + this->PopLoopBlockBarrier(); + cmDefinitions* current = &this->Internal->VarStack.top(); std::set init = this->Internal->VarInitStack.top(); std::set usage = this->Internal->VarUsageStack.top(); diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 0458d5477..41dddc303 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -34,6 +34,8 @@ # include #endif +#include + 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. @@ -900,6 +911,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 { @@ -960,6 +975,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); @@ -1054,6 +1073,8 @@ private: void PushFunctionBlockerBarrier(); void PopFunctionBlockerBarrier(bool reportError = true); + std::stack LoopBlockCounter; + typedef std::map StringStringMap; StringStringMap MacrosMap; diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx index a420f5958..64b87b730 100644 --- a/Source/cmPolicies.cxx +++ b/Source/cmPolicies.cxx @@ -364,6 +364,11 @@ cmPolicies::cmPolicies() CMP0054, "CMP0054", "Only interpret if() arguments as variables or keywords when unquoted.", 3,1,0, cmPolicies::WARN); + + this->DefinePolicy( + CMP0055, "CMP0055", + "Strict checking for break() command.", + 3,2,0, cmPolicies::WARN); } cmPolicies::~cmPolicies() diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index 7c73da82b..46ecc22ad 100644 --- a/Source/cmPolicies.h +++ b/Source/cmPolicies.h @@ -111,6 +111,7 @@ public: CMP0053, ///< Simplify variable reference and escape sequence evaluation CMP0054, ///< Only interpret if() arguments as variables /// or keywords when unquoted. + CMP0055, ///< Strict checking for break() command. /** \brief Always the last entry. * diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx index 851c4cb20..d36095e6c 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 fb(mf.RemoveFunctionBlocker(this, lff)); @@ -138,6 +140,8 @@ bool cmWhileCommand f->Args = args; this->Makefile->AddFunctionBlocker(f); + this->Makefile->PushLoopBlock(); + return true; } diff --git a/Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope-result.txt b/Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope-stderr.txt b/Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope-stderr.txt new file mode 100644 index 000000000..27e81405d --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope-stderr.txt @@ -0,0 +1,4 @@ +CMake Error at CMP0055-NEW-Out-of-Scope.cmake:4 \(break\): + A BREAK command was found outside of a proper FOREACH or WHILE loop scope. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope.cmake b/Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope.cmake new file mode 100644 index 000000000..53ac214ad --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope.cmake @@ -0,0 +1,4 @@ + +cmake_policy(SET CMP0055 NEW) + +break() diff --git a/Tests/RunCMake/CMP0055/CMP0055-NEW-Reject-Arguments-result.txt b/Tests/RunCMake/CMP0055/CMP0055-NEW-Reject-Arguments-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-NEW-Reject-Arguments-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/CMP0055/CMP0055-NEW-Reject-Arguments-stderr.txt b/Tests/RunCMake/CMP0055/CMP0055-NEW-Reject-Arguments-stderr.txt new file mode 100644 index 000000000..32947af2a --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-NEW-Reject-Arguments-stderr.txt @@ -0,0 +1,4 @@ +CMake Error at CMP0055-NEW-Reject-Arguments.cmake:5 \(break\): + The BREAK command does not accept any arguments. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/CMP0055/CMP0055-NEW-Reject-Arguments.cmake b/Tests/RunCMake/CMP0055/CMP0055-NEW-Reject-Arguments.cmake new file mode 100644 index 000000000..52eaa6aa5 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-NEW-Reject-Arguments.cmake @@ -0,0 +1,6 @@ + +cmake_policy(SET CMP0055 NEW) + +foreach(i RANGE 1 2) + break(1) +endforeach() \ No newline at end of file diff --git a/Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope-result.txt b/Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope-result.txt new file mode 100644 index 000000000..573541ac9 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope-result.txt @@ -0,0 +1 @@ +0 diff --git a/Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope-stderr.txt b/Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope-stderr.txt new file mode 100644 index 000000000..10f32932e --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope-stderr.txt @@ -0,0 +1 @@ +^$ diff --git a/Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope.cmake b/Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope.cmake new file mode 100644 index 000000000..57195c248 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope.cmake @@ -0,0 +1,4 @@ + +cmake_policy(SET CMP0055 OLD) + +break() diff --git a/Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments-result.txt b/Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments-result.txt new file mode 100644 index 000000000..573541ac9 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments-result.txt @@ -0,0 +1 @@ +0 diff --git a/Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments-stderr.txt b/Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments-stderr.txt new file mode 100644 index 000000000..10f32932e --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments-stderr.txt @@ -0,0 +1 @@ +^$ diff --git a/Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments.cmake b/Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments.cmake new file mode 100644 index 000000000..d8fdddf10 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments.cmake @@ -0,0 +1,6 @@ + +cmake_policy(SET CMP0055 OLD) + +foreach(i RANGE 1 2) + break(1) +endforeach() \ No newline at end of file diff --git a/Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope-result.txt b/Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope-result.txt new file mode 100644 index 000000000..573541ac9 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope-result.txt @@ -0,0 +1 @@ +0 diff --git a/Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope-stderr.txt b/Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope-stderr.txt new file mode 100644 index 000000000..ad850ac93 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope-stderr.txt @@ -0,0 +1,9 @@ +CMake Warning \(dev\) at CMP0055-WARN-Out-of-Scope.cmake:2 \(break\): + Policy CMP0055 is not set: Strict checking for break\(\) command. Run "cmake + --help-policy CMP0055" for policy details. Use the cmake_policy command to + set the policy and suppress this warning. + + A BREAK command was found outside of a proper FOREACH or WHILE loop scope. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) +This warning is for project developers. Use -Wno-dev to suppress it. diff --git a/Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope.cmake b/Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope.cmake new file mode 100644 index 000000000..373a95a7c --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope.cmake @@ -0,0 +1,2 @@ + +break() diff --git a/Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments-result.txt b/Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments-result.txt new file mode 100644 index 000000000..573541ac9 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments-result.txt @@ -0,0 +1 @@ +0 diff --git a/Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments-stderr.txt b/Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments-stderr.txt new file mode 100644 index 000000000..3cc686dc0 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments-stderr.txt @@ -0,0 +1,9 @@ +CMake Warning \(dev\) at CMP0055-WARN-Reject-Arguments.cmake:3 \(break\): + Policy CMP0055 is not set: Strict checking for break\(\) command. Run "cmake + --help-policy CMP0055" for policy details. Use the cmake_policy command to + set the policy and suppress this warning. + + The BREAK command does not accept any arguments. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) +This warning is for project developers. Use -Wno-dev to suppress it. diff --git a/Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments.cmake b/Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments.cmake new file mode 100644 index 000000000..ec6b90f8a --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments.cmake @@ -0,0 +1,4 @@ + +foreach(i RANGE 1 2) + break(1) +endforeach() diff --git a/Tests/RunCMake/CMP0055/CMakeLists.txt b/Tests/RunCMake/CMP0055/CMakeLists.txt new file mode 100644 index 000000000..ef2163c29 --- /dev/null +++ b/Tests/RunCMake/CMP0055/CMakeLists.txt @@ -0,0 +1,3 @@ +cmake_minimum_required(VERSION 3.1) +project(${RunCMake_TEST} NONE) +include(${RunCMake_TEST}.cmake) diff --git a/Tests/RunCMake/CMP0055/RunCMakeTest.cmake b/Tests/RunCMake/CMP0055/RunCMakeTest.cmake new file mode 100644 index 000000000..efcfcab67 --- /dev/null +++ b/Tests/RunCMake/CMP0055/RunCMakeTest.cmake @@ -0,0 +1,9 @@ +include(RunCMake) + +run_cmake(CMP0055-OLD-Out-of-Scope) +run_cmake(CMP0055-NEW-Out-of-Scope) +run_cmake(CMP0055-WARN-Out-of-Scope) + +run_cmake(CMP0055-OLD-Reject-Arguments) +run_cmake(CMP0055-NEW-Reject-Arguments) +run_cmake(CMP0055-WARN-Reject-Arguments) diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index 8e6bad40c..9832a9510 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -49,6 +49,7 @@ add_RunCMake_test(CMP0050) add_RunCMake_test(CMP0051) add_RunCMake_test(CMP0053) add_RunCMake_test(CMP0054) +add_RunCMake_test(CMP0055) add_RunCMake_test(CTest) if(UNIX AND "${CMAKE_GENERATOR}" MATCHES "Unix Makefiles|Ninja") add_RunCMake_test(CompilerChange)