break: Add policy CMP0055 to check calls strictly

Reject break() without loop scope or any arguments.

Signed-off-by: Gregor Jasny <[email protected]>

Help/manual/cmake-policies.7.rst

@@ -112,3 +112,4 @@ All Policies
    /policy/CMP0052
    /policy/CMP0053
    /policy/CMP0054
+   /policy/CMP0055

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.

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`.

Source/cmBreakCommand.cxx

@@ -12,10 +12,76 @@
 #include "cmBreakCommand.h"
 
 // cmBreakCommand
-bool cmBreakCommand::InitialPass(std::vector<std::string> const&,
+bool cmBreakCommand::InitialPass(std::vector<std::string> 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;
 }
 

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()

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.
      *

Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope-result.txt

@@ -0,0 +1 @@
+1

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\)

Tests/RunCMake/CMP0055/CMP0055-NEW-Out-of-Scope.cmake

@@ -0,0 +1,4 @@
+
+cmake_policy(SET CMP0055 NEW)
+
+break()

Tests/RunCMake/CMP0055/CMP0055-NEW-Reject-Arguments-result.txt

@@ -0,0 +1 @@
+1

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\)

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()

Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope-result.txt

@@ -0,0 +1 @@
+0

Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope-stderr.txt

@@ -0,0 +1 @@
+^$

Tests/RunCMake/CMP0055/CMP0055-OLD-Out-of-Scope.cmake

@@ -0,0 +1,4 @@
+
+cmake_policy(SET CMP0055 OLD)
+
+break()

Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments-result.txt

@@ -0,0 +1 @@
+0

Tests/RunCMake/CMP0055/CMP0055-OLD-Reject-Arguments-stderr.txt

@@ -0,0 +1 @@
+^$

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()

Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope-result.txt

@@ -0,0 +1 @@
+0

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.

Tests/RunCMake/CMP0055/CMP0055-WARN-Out-of-Scope.cmake

@@ -0,0 +1,2 @@
+
+break()

Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments-result.txt

@@ -0,0 +1 @@
+0

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.

Tests/RunCMake/CMP0055/CMP0055-WARN-Reject-Arguments.cmake

@@ -0,0 +1,4 @@
+
+foreach(i RANGE 1 2)
+  break(1)
+endforeach()

Tests/RunCMake/CMP0055/CMakeLists.txt

@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 3.1)
+project(${RunCMake_TEST} NONE)
+include(${RunCMake_TEST}.cmake)

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)

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)