while: diagnose errors during condition evaluation

Add a policy to diagnose condition errors in a compatible way.

Fixes: #23296

Help/manual/cmake-policies.7.rst

@@ -52,6 +52,14 @@ to determine whether to report an error on use of deprecated macros or
 functions.
 
 
+Policies Introduced by CMake 3.24
+=================================
+
+.. toctree::
+   :maxdepth: 1
+
+   CMP0130: while() diagnoses condition evaluation errors. </policy/CMP0130>
+
 Policies Introduced by CMake 3.23
 =================================
 

Help/policy/CMP0130.rst

@@ -0,0 +1,32 @@
+CMP0130
+-------
+
+.. versionadded:: 3.24
+
+:command:`while` diagnoses condition evaluation errors.
+
+CMake 3.23 and below accidentally tolerated errors encountered while
+evaluating the condition passed to the :command:`while` command
+(but not the :command:`if` command).  For example, the code
+
+.. code-block:: cmake
+
+  set(paren "(")
+  while(${paren})
+  endwhile()
+
+creates an unbalanced parenthesis during condition evaluation.
+
+CMake 3.24 and above prefer to diagnose such errors.  This policy
+provides compatibility for projects that have not been updated to
+fix their condition errors.
+
+The ``OLD`` behavior for this policy is to ignore errors in
+:command:`while` conditions.  The ``NEW`` behavior for this
+policy is to diagnose errors in :command:`while` conditions.
+
+This policy was introduced in CMake version 3.24.  CMake version |release|
+warns when the policy is not set and uses ``OLD`` behavior.  Use the
+:command:`cmake_policy` command to set it to ``OLD`` or ``NEW`` explicitly.
+
+.. include:: DEPRECATED.txt

Help/release/dev/while-errors.rst

@@ -0,0 +1,5 @@
+while-errors
+------------
+
+* The :command:`while` command now diagnoses errors during condition
+  evaluation.  See policy :policy:`CMP0130`.

Source/cmPolicies.h

@@ -388,7 +388,9 @@ class cmMakefile;
          22, 0, cmPolicies::WARN)                                             \
   SELECT(POLICY, CMP0129,                                                     \
          "Compiler id for MCST LCC compilers is now LCC, not GNU.", 3, 23, 0, \
-         cmPolicies::WARN)
+         cmPolicies::WARN)                                                    \
+  SELECT(POLICY, CMP0130, "while() diagnoses condition evaluation errors.",   \
+         3, 24, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \

Source/cmWhileCommand.cxx

@@ -17,6 +17,8 @@
 #include "cmMakefile.h"
 #include "cmMessageType.h"
 #include "cmOutputConverter.h"
+#include "cmPolicies.h"
+#include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
 #include "cmake.h"
 
@@ -79,9 +81,8 @@ bool cmWhileFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
     return out;
   };
 
-  // FIXME(#23296): For compatibility with older versions of CMake, we
-  // tolerate condition errors that evaluate to false.  We should add
-  // a policy to enforce such errors.
+  // For compatibility with projects that do not set CMP0130 to NEW,
+  // we tolerate condition errors that evaluate to false.
   bool enforceError = true;
   std::string errorString;
   MessageType messageType;
@@ -110,14 +111,38 @@ bool cmWhileFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
     }
   }
 
+  if (!errorString.empty() && !enforceError) {
+    // This error should only be enforced if CMP0130 is NEW.
+    switch (mf.GetPolicyStatus(cmPolicies::CMP0130)) {
+      case cmPolicies::WARN:
+        // Convert the error to a warning and enforce it.
+        messageType = MessageType::AUTHOR_WARNING;
+        enforceError = true;
+        break;
+      case cmPolicies::OLD:
+        // OLD behavior is to silently ignore the error.
+        break;
+      case cmPolicies::REQUIRED_ALWAYS:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::NEW:
+        // NEW behavior is to enforce the error.
+        enforceError = true;
+        break;
+    }
+  }
+
   if (!errorString.empty() && enforceError) {
-    std::string err = "had incorrect arguments:\n ";
+    std::string err = "while() given incorrect arguments:\n ";
     for (auto const& i : expandedArguments) {
       err += " ";
       err += cmOutputConverter::EscapeForCMake(i.GetValue());
     }
     err += "\n";
     err += errorString;
+    if (mf.GetPolicyStatus(cmPolicies::CMP0130) == cmPolicies::WARN) {
+      err =
+        cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0130), '\n', err);
+    }
     mf.GetCMakeInstance()->IssueMessage(messageType, err, whileBT);
     if (messageType == MessageType::FATAL_ERROR) {
       cmSystemTools::SetFatalErrorOccured();

Tests/RunCMake/while/CMP0130-NEW-result.txt

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

Tests/RunCMake/while/CMP0130-NEW-stderr.txt

@@ -0,0 +1,9 @@
+^CMake Error at CMP0130-common.cmake:[0-9]+ \(while\):
+  while\(\) given incorrect arguments:
+
+    "\("
+
+  mismatched parenthesis in condition
+Call Stack \(most recent call first\):
+  CMP0130-NEW.cmake:[0-9]+ \(include\)
+  CMakeLists.txt:[0-9]+ \(include\)$

Tests/RunCMake/while/CMP0130-NEW.cmake

@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0130 NEW)
+include(CMP0130-common.cmake)

Tests/RunCMake/while/CMP0130-OLD.cmake

@@ -0,0 +1,2 @@
+cmake_policy(SET CMP0130 OLD)
+include(CMP0130-common.cmake)

Tests/RunCMake/while/CMP0130-WARN-stderr.txt

@@ -0,0 +1,14 @@
+^CMake Warning \(dev\) at CMP0130-common.cmake:[0-9]+ \(while\):
+  Policy CMP0130 is not set: while\(\) diagnoses condition evaluation errors.
+  Run "cmake --help-policy CMP0130" for policy details.  Use the cmake_policy
+  command to set the policy and suppress this warning.
+
+  while\(\) given incorrect arguments:
+
+    "\("
+
+  mismatched parenthesis in condition
+Call Stack \(most recent call first\):
+  CMP0130-WARN.cmake:[0-9]+ \(include\)
+  CMakeLists.txt:[0-9]+ \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.$

Tests/RunCMake/while/CMP0130-WARN-stdout.txt

@@ -0,0 +1 @@
+-- Code incorrectly accepted

Tests/RunCMake/while/CMP0130-WARN.cmake

@@ -0,0 +1,2 @@
+# CMP0130 left unset
+include(CMP0130-common.cmake)

Tests/RunCMake/while/unbalanced-parenthesis.cmake → Tests/RunCMake/while/CMP0130-common.cmake

@@ -3,5 +3,4 @@ while(${paren})
   message(STATUS "Condition incorrectly true")
   break()
 endwhile()
-# FIXME(#23296): The above condition error is tolerated for compatibility.
 message(STATUS "Code incorrectly accepted")

Tests/RunCMake/while/RunCMakeTest.cmake

@@ -6,4 +6,6 @@ run_cmake(EndMismatch)
 run_cmake(EndAlone)
 run_cmake(EndAloneArgs)
 
-run_cmake(unbalanced-parenthesis)
+run_cmake(CMP0130-OLD)
+run_cmake(CMP0130-WARN)
+run_cmake(CMP0130-NEW)