mirror/CMake
1
0
Fork 0
mirror of https://github.com/Kitware/CMake.git synced 2026-01-07 14:20:06 -06:00
Code Issues Packages Projects Releases Wiki Activity

Merge topic 'cmake-parse-arguments-one-arg-empty-string'

Browse Source 
ceeea4e511 cmake_parse_arguments: Set variable if empty string given after keyword
2f5cc6afa1 cmParseArgumentsCommand: Use cmStrCat() for string concatenation

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !9748
This commit is contained in:
Brad King
2024-08-26 13:52:51 +00:00
committed by Kitware Robot
parent de05bc2f15 ceeea4e511
commit d625e832ed
8 changed files with 152 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
Help/command/cmake_parse_arguments.rst
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:

1
Help/manual/cmake-policies.7.rst
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

75
Source/cmParseArgumentsCommand.cxx
View File

@@ -4,9 +4,9 @@
#include <map>
#include <set>
#include <sstream>
#include <utility>
#include <cm/optional>
#include <cm/string_view>
#include "cmArgumentParser.h"
@@ -15,6 +15,7 @@
#include "cmList.h"
#include "cmMakefile.h"
#include "cmMessageType.h"
#include "cmPolicies.h"
#include "cmRange.h"
#include "cmStringAlgorithms.h"
#include "cmSystemTools.h"
@@ -42,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>;
@@ -79,40 +80,60 @@ static void PassParsedArguments(
const options_set& keywordsMissingValues, bool parseFromArgV)
{
for (auto const& iter : options) {
makefile.AddDefinition(prefix + iter.first,
makefile.AddDefinition(cmStrCat(prefix, iter.first),
iter.second ? "TRUE" : "FALSE");
}
const cmPolicies::PolicyStatus cmp0174 =
makefile.GetPolicyStatus(cmPolicies::CMP0174);
for (auto const& iter : singleValArgs) {
if (!iter.second.empty()) {
makefile.AddDefinition(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) {
if (!iter.second.empty()) {
makefile.AddDefinition(prefix + iter.first,
makefile.AddDefinition(cmStrCat(prefix, iter.first),
JoinList(iter.second, parseFromArgV));
} else {
makefile.RemoveDefinition(prefix + iter.first);
makefile.RemoveDefinition(cmStrCat(prefix, iter.first));
}
}
if (!unparsed.empty()) {
makefile.AddDefinition(prefix + "UNPARSED_ARGUMENTS",
makefile.AddDefinition(cmStrCat(prefix, "UNPARSED_ARGUMENTS"),
JoinList(unparsed, parseFromArgV));
} else {
makefile.RemoveDefinition(prefix + "UNPARSED_ARGUMENTS");
makefile.RemoveDefinition(cmStrCat(prefix, "UNPARSED_ARGUMENTS"));
}
if (!keywordsMissingValues.empty()) {
makefile.AddDefinition(
prefix + "KEYWORDS_MISSING_VALUES",
cmStrCat(prefix, "KEYWORDS_MISSING_VALUES"),
cmList::to_string(cmMakeRange(keywordsMissingValues)));
} else {
makefile.RemoveDefinition(prefix + "KEYWORDS_MISSING_VALUES");
makefile.RemoveDefinition(cmStrCat(prefix, "KEYWORDS_MISSING_VALUES"));
}
}
@@ -143,9 +164,10 @@ bool cmParseArgumentsCommand(std::vector<std::string> const& args,
parseFromArgV = true;
argIter++; // move past PARSE_ARGV
if (!cmStrToULong(*argIter, &argvStart)) {
status.GetMakefile().IssueMessage(MessageType::FATAL_ERROR,
"PARSE_ARGV index '" + *argIter +
"' is not an unsigned integer");
status.GetMakefile().IssueMessage(
MessageType::FATAL_ERROR,
cmStrCat("PARSE_ARGV index '", *argIter,
"' is not an unsigned integer"));
cmSystemTools::SetFatalErrorOccurred();
return true;
}
@@ -167,7 +189,7 @@ bool cmParseArgumentsCommand(std::vector<std::string> const& args,
auto const duplicateKey = [&status](std::string const& key) {
status.GetMakefile().IssueMessage(
MessageType::WARNING, "keyword defined more than once: " + key);
MessageType::WARNING, cmStrCat("keyword defined more than once: ", key));
};
// the second argument is a (cmake) list of options without argument
@@ -194,21 +216,20 @@ bool cmParseArgumentsCommand(std::vector<std::string> const& args,
std::string argc = status.GetMakefile().GetSafeDefinition("ARGC");
unsigned long count;
if (!cmStrToULong(argc, &count)) {
status.GetMakefile().IssueMessage(MessageType::FATAL_ERROR,
"PARSE_ARGV called with ARGC='" +
argc +
"' that is not an unsigned integer");
status.GetMakefile().IssueMessage(
MessageType::FATAL_ERROR,
cmStrCat("PARSE_ARGV called with ARGC='", argc,
"' that is not an unsigned integer"));
cmSystemTools::SetFatalErrorOccurred();
return true;
}
for (unsigned long i = argvStart; i < count; ++i) {
std::ostringstream argName;
argName << "ARGV" << i;
cmValue arg = status.GetMakefile().GetDefinition(argName.str());
const std::string argName{ cmStrCat("ARGV", i) };
cmValue arg = status.GetMakefile().GetDefinition(argName);
if (!arg) {
status.GetMakefile().IssueMessage(MessageType::FATAL_ERROR,
"PARSE_ARGV called with " +
argName.str() + " not set");
status.GetMakefile().IssueMessage(
MessageType::FATAL_ERROR,
cmStrCat("PARSE_ARGV called with ", argName, " not set"));
cmSystemTools::SetFatalErrorOccurred();
return true;
}

6
Source/cmPolicies.h
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) \

11
Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN-stderr.txt Normal file
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\.

45
Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN.cmake
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()

4
Tests/RunCMake/cmake_parse_arguments/test_utils.cmake
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()