1
0
mirror of https://github.com/Kitware/CMake.git synced 2025-10-15 03:48:02 +08:00

cmake_parse_arguments: Set variable if empty string given after keyword

If a single-value keyword is followed by an empty string, the
command unsets the variable for that keyword instead of setting
it to the empty string. This is inconsistent and unexpected. Add
policy CMP0174 which ensures the variable for a single-value
keyword is always set when any value is given, not just for a
non-empty value.

The new CMP0174 policy only affects the PARSE_ARGV form of
cmake_parse_arguments. The older form silently drops all empty
string arguments before processing the argument list.

Fixes: #25972
This commit is contained in:
Craig Scott
2024-08-18 22:38:02 +10:00
parent 2f5cc6afa1
commit ceeea4e511
8 changed files with 131 additions and 7 deletions

View File

@@ -70,6 +70,12 @@ whether your macro was called with unrecognized parameters.
received values. This can be checked to see if there were keywords without
any values given.
.. versionchanged:: 3.31
If a ``<one_value_keyword>`` is followed by an empty string as its value,
policy :policy:`CMP0174` controls whether a corresponding
``<prefix>_<keyword>`` variable is defined or not.
Consider the following example macro, ``my_install()``, which takes similar
arguments to the real :command:`install` command:

View File

@@ -57,6 +57,7 @@ Policies Introduced by CMake 3.31
.. toctree::
:maxdepth: 1
CMP0174: cmake_parse_arguments(PARSE_ARGV) defines a variable for an empty string after a single-value keyword. </policy/CMP0174>
CMP0173: The CMakeFindFrameworks module is removed. </policy/CMP0173>
CMP0172: The CPack module enables per-machine installation by default in the CPack WIX Generator. </policy/CMP0172>
CMP0171: 'codegen' is a reserved target name. </policy/CMP0171>

33
Help/policy/CMP0174.rst Normal file
View File

@@ -0,0 +1,33 @@
CMP0174
-------
.. versionadded:: 3.31
:command:`cmake_parse_arguments(PARSE_ARGV)` defines a variable for an empty
string after a single-value keyword.
One of the main reasons for using the ``PARSE_ARGV`` form of the
:command:`cmake_parse_arguments` command is to more robustly handle corner
cases related to empty values. The non-``PARSE_ARGV`` form doesn't preserve
empty arguments, but the ``PARSE_ARGV`` form does. For each single-value
keyword given, a variable should be defined if the keyword is followed by a
value. Prior to CMake 3.31, no variable would be defined if the value given
was an empty string. This meant the code could not detect the difference
between the keyword not being given, and it being given but with an empty
value, except by iterating over all the arguments and checking if the keyword
is present.
For the ``OLD`` behavior of this policy,
:command:`cmake_parse_arguments(PARSE_ARGV)` does not define a variable for a
single-value keyword followed by an empty string.
For the ``NEW`` behavior, :command:`cmake_parse_arguments(PARSE_ARGV)` always
defines a variable for each keyword given in the arguments, even a single-value
keyword with an empty string as its value. With the ``NEW`` behavior, the
code can robustly check if a single-value keyword was given with any value
using just ``if(DEFINED <prefix>_<keyword>)``.
.. |INTRODUCED_IN_CMAKE_VERSION| replace:: 3.31
.. |WARNS_OR_DOES_NOT_WARN| replace:: warns
.. include:: STANDARD_ADVICE.txt
.. include:: DEPRECATED.txt

View File

@@ -6,6 +6,7 @@
#include <set>
#include <utility>
#include <cm/optional>
#include <cm/string_view>
#include "cmArgumentParser.h"
@@ -14,6 +15,7 @@
#include "cmList.h"
#include "cmMakefile.h"
#include "cmMessageType.h"
#include "cmPolicies.h"
#include "cmRange.h"
#include "cmStringAlgorithms.h"
#include "cmSystemTools.h"
@@ -41,7 +43,7 @@ std::string JoinList(std::vector<std::string> const& arg, bool escape)
}
using options_map = std::map<std::string, bool>;
using single_map = std::map<std::string, std::string>;
using single_map = std::map<std::string, cm::optional<std::string>>;
using multi_map =
std::map<std::string, ArgumentParser::NonEmpty<std::vector<std::string>>>;
using options_set = std::set<cm::string_view>;
@@ -82,12 +84,32 @@ static void PassParsedArguments(
iter.second ? "TRUE" : "FALSE");
}
const cmPolicies::PolicyStatus cmp0174 =
makefile.GetPolicyStatus(cmPolicies::CMP0174);
for (auto const& iter : singleValArgs) {
if (!iter.second.empty()) {
makefile.AddDefinition(cmStrCat(prefix, iter.first), iter.second);
} else {
makefile.RemoveDefinition(prefix + iter.first);
if (iter.second.has_value()) {
// So far, we only know that the keyword was given, not whether a value
// was provided after the keyword
if (keywordsMissingValues.find(iter.first) ==
keywordsMissingValues.end()) {
// A (possibly empty) value was given
if (cmp0174 == cmPolicies::NEW || !iter.second->empty()) {
makefile.AddDefinition(cmStrCat(prefix, iter.first), *iter.second);
continue;
}
if (cmp0174 == cmPolicies::WARN) {
makefile.IssueMessage(
MessageType::AUTHOR_WARNING,
cmStrCat("An empty string was given as the value after the ",
iter.first,
" keyword. Policy CMP0174 is not set, so "
"cmake_parse_arguments() will unset the ",
prefix, iter.first,
" variable rather than setting it to an empty string."));
}
}
}
makefile.RemoveDefinition(prefix + iter.first);
}
for (auto const& iter : multiValArgs) {

View File

@@ -533,7 +533,11 @@ class cmMakefile;
"the CPack WIX Generator.", \
3, 31, 0, cmPolicies::WARN) \
SELECT(POLICY, CMP0173, "The CMakeFindFrameworks module is removed.", 3, \
31, 0, cmPolicies::WARN)
31, 0, cmPolicies::WARN) \
SELECT(POLICY, CMP0174, \
"cmake_parse_arguments(PARSE_ARGV) defines a variable for an empty " \
"string after a single-value keyword.", \
3, 31, 0, cmPolicies::WARN)
#define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
#define CM_FOR_EACH_POLICY_ID(POLICY) \

View File

@@ -0,0 +1,11 @@
Testing CMP0174 = NEW
Testing CMP0174 = OLD
Testing CMP0174 = WARN
CMake Warning \(dev\) at CornerCasesArgvN\.cmake:[0-9]+ \(cmake_parse_arguments\):
An empty string was given as the value after the P1 keyword\. Policy
CMP0174 is not set, so cmake_parse_arguments\(\) will unset the arg_P1
variable rather than setting it to an empty string\.
Call Stack \(most recent call first\):
CornerCasesArgvN\.cmake:[0-9]+ \(test_cmp0174_warn\)
CMakeLists\.txt:[0-9]+ \(include\)
This warning is for project developers\. Use -Wno-dev to suppress it\.

View File

@@ -51,3 +51,48 @@ function(test4)
TEST(upref_UNPARSED_ARGUMENTS foo "bar;none")
endfunction()
test4(MULTI foo bar\\ none)
# Single-value keyword with empty string as value
message(NOTICE "Testing CMP0174 = NEW")
block(SCOPE_FOR POLICIES)
cmake_policy(SET CMP0174 NEW)
function(test_cmp0174_new_missing)
cmake_parse_arguments(PARSE_ARGV 0 arg "" "P1;P2" "")
TEST(arg_KEYWORDS_MISSING_VALUES "P2")
TEST(arg_P1 "")
TEST(arg_P2 "UNDEFINED")
endfunction()
test_cmp0174_new_missing(P1 "" P2)
function(test_cmp0174_new_no_missing)
cmake_parse_arguments(PARSE_ARGV 0 arg "" "P1;P2" "")
TEST(arg_KEYWORDS_MISSING_VALUES "UNDEFINED")
TEST(arg_P1 "")
TEST(arg_P2 "UNDEFINED")
endfunction()
test_cmp0174_new_no_missing(P1 "")
endblock()
message(NOTICE "Testing CMP0174 = OLD")
block(SCOPE_FOR POLICIES)
cmake_policy(SET CMP0174 OLD)
function(test_cmp0174_old)
cmake_parse_arguments(PARSE_ARGV 0 arg "" "P1;P2" "")
TEST(arg_KEYWORDS_MISSING_VALUES "P2")
TEST(arg_P1 "UNDEFINED")
TEST(arg_P2 "UNDEFINED")
endfunction()
test_cmp0174_old(P1 "" P2)
endblock()
message(NOTICE "Testing CMP0174 = WARN")
block(SCOPE_FOR POLICIES)
cmake_policy(VERSION 3.30) # WARN
function(test_cmp0174_warn)
cmake_parse_arguments(PARSE_ARGV 0 arg "" "P1;P2" "")
TEST(arg_KEYWORDS_MISSING_VALUES "P2")
TEST(arg_P1 "UNDEFINED")
TEST(arg_P2 "UNDEFINED")
endfunction()
test_cmp0174_warn(P1 "" P2)
endblock()

View File

@@ -13,6 +13,8 @@ function(TEST variable)
if(DEFINED ${variable})
message(FATAL_ERROR "'${variable}' shall be undefined but has value '${${variable}}'")
endif()
elseif(NOT DEFINED ${variable})
message(FATAL_ERROR "'${variable}' should be defined but is not")
elseif("${expected}" STREQUAL "FALSE")
if(NOT ${variable} STREQUAL "FALSE")
message(FATAL_ERROR "'${variable}' shall be FALSE")
@@ -23,7 +25,7 @@ function(TEST variable)
endif()
else()
if(NOT ${variable} STREQUAL "${expected}")
message(FATAL_ERROR "'${variable}' shall be '${expected}'")
message(FATAL_ERROR "'${variable}' shall be '${expected}' but has value '${${variable}}'")
endif()
endif()
endif()