diff options
| -rw-r--r-- | Help/policy/CMP0174.rst | 24 | ||||
| -rw-r--r-- | Source/cmParseArgumentsCommand.cxx | 54 | ||||
| -rw-r--r-- | Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN-stderr.txt | 2 | ||||
| -rw-r--r-- | Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN.cmake | 34 |
4 files changed, 75 insertions, 39 deletions
diff --git a/Help/policy/CMP0174.rst b/Help/policy/CMP0174.rst index bde0a03..748fdba 100644 --- a/Help/policy/CMP0174.rst +++ b/Help/policy/CMP0174.rst @@ -10,21 +10,25 @@ 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. +keyword given, a variable should be defined if the keyword is present, even +if it is followed by an empty string. + +Prior to CMake 3.31, no variable would be defined if the value given after a +single-value keyword 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. +single-value keyword followed by an empty string, or followed by no value at +all. + 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>)``. +keyword with an empty string as its value or no value at all. With the +``NEW`` behavior, the code can robustly check if a single-value keyword was +given using just ``if(DEFINED <prefix>_<keyword>)``. .. |INTRODUCED_IN_CMAKE_VERSION| replace:: 3.31 .. |WARNS_OR_DOES_NOT_WARN| replace:: warns diff --git a/Source/cmParseArgumentsCommand.cxx b/Source/cmParseArgumentsCommand.cxx index f6e7937..40b18ff 100644 --- a/Source/cmParseArgumentsCommand.cxx +++ b/Source/cmParseArgumentsCommand.cxx @@ -6,7 +6,6 @@ #include <set> #include <utility> -#include <cm/optional> #include <cm/string_view> #include "cmArgumentParser.h" @@ -43,7 +42,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, cm::optional<std::string>>; +using single_map = std::map<std::string, std::string>; using multi_map = std::map<std::string, ArgumentParser::NonEmpty<std::vector<std::string>>>; using options_set = std::set<cm::string_view>; @@ -76,7 +75,7 @@ struct UserArgumentParser : public cmArgumentParser<void> static void PassParsedArguments( const std::string& prefix, cmMakefile& makefile, const options_map& options, const single_map& singleValArgs, const multi_map& multiValArgs, - const std::vector<std::string>& unparsed, + const std::vector<std::string>& unparsed, const options_set& keywordsSeen, const options_set& keywordsMissingValues, bool parseFromArgV) { for (auto const& iter : options) { @@ -87,29 +86,26 @@ static void PassParsedArguments( const cmPolicies::PolicyStatus cmp0174 = makefile.GetPolicyStatus(cmPolicies::CMP0174); for (auto const& iter : singleValArgs) { - 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.")); - } + if (keywordsSeen.find(iter.first) == keywordsSeen.end()) { + makefile.RemoveDefinition(cmStrCat(prefix, iter.first)); + } else if ((parseFromArgV && cmp0174 == cmPolicies::NEW) || + !iter.second.empty()) { + makefile.AddDefinition(cmStrCat(prefix, iter.first), iter.second); + } else { + // The OLD policy behavior doesn't define a variable for an empty or + // missing value, and we can't differentiate between those two cases. + if (parseFromArgV && (cmp0174 == cmPolicies::WARN)) { + makefile.IssueMessage( + MessageType::AUTHOR_WARNING, + cmStrCat("The ", iter.first, + " keyword was followed by an empty string or no value at " + "all. 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(cmStrCat(prefix, iter.first)); } - makefile.RemoveDefinition(prefix + iter.first); } for (auto const& iter : multiValArgs) { @@ -237,6 +233,14 @@ bool cmParseArgumentsCommand(std::vector<std::string> const& args, } } + std::vector<cm::string_view> keywordsSeen; + parser.BindParsedKeywords(keywordsSeen); + + // For single-value keywords, only the last instance matters, since it + // determines the value stored. But if a keyword is repeated, it will be + // added to this vector if _any_ instance is missing a value. If one of the + // earlier instances is missing a value but the last one isn't, its presence + // in this vector will be misleading. std::vector<cm::string_view> keywordsMissingValues; parser.BindKeywordsMissingValue(keywordsMissingValues); @@ -244,7 +248,7 @@ bool cmParseArgumentsCommand(std::vector<std::string> const& args, PassParsedArguments( prefix, status.GetMakefile(), options, singleValArgs, multiValArgs, - unparsed, + unparsed, options_set(keywordsSeen.begin(), keywordsSeen.end()), options_set(keywordsMissingValues.begin(), keywordsMissingValues.end()), parseFromArgV); diff --git a/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN-stderr.txt b/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN-stderr.txt index 1b87306..0815ed3 100644 --- a/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN-stderr.txt +++ b/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN-stderr.txt @@ -2,7 +2,7 @@ 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 + The P1 keyword was followed by an empty string or no value at all\. 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\): diff --git a/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN.cmake b/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN.cmake index 04c0219..487a48f 100644 --- a/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN.cmake +++ b/Tests/RunCMake/cmake_parse_arguments/CornerCasesArgvN.cmake @@ -60,7 +60,7 @@ block(SCOPE_FOR POLICIES) cmake_parse_arguments(PARSE_ARGV 0 arg "" "P1;P2" "") TEST(arg_KEYWORDS_MISSING_VALUES "P2") TEST(arg_P1 "") - TEST(arg_P2 "UNDEFINED") + TEST(arg_P2 "") endfunction() test_cmp0174_new_missing(P1 "" P2) @@ -71,28 +71,56 @@ block(SCOPE_FOR POLICIES) TEST(arg_P2 "UNDEFINED") endfunction() test_cmp0174_new_no_missing(P1 "") + + # For repeated keywords, the keyword will be reported as missing a value if + # any one of them is missing a value. + function(test_cmp0174_new_repeated_arg) + cmake_parse_arguments(PARSE_ARGV 0 arg "" "P1;P2" "") + TEST(arg_KEYWORDS_MISSING_VALUES "P1") + TEST(arg_P1 "abc") + TEST(arg_P2 "UNDEFINED") + endfunction() + test_cmp0174_new_repeated_arg(P1 P1 abc) 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" "") + cmake_parse_arguments(PARSE_ARGV 0 arg "" "P1;P2;P3" "") TEST(arg_KEYWORDS_MISSING_VALUES "P2") TEST(arg_P1 "UNDEFINED") TEST(arg_P2 "UNDEFINED") + TEST(arg_P3 "UNDEFINED") endfunction() test_cmp0174_old(P1 "" P2) + + function(test_cmp0174_old_repeated_arg) + cmake_parse_arguments(PARSE_ARGV 0 arg "" "P1;P2" "") + TEST(arg_KEYWORDS_MISSING_VALUES "P1") + TEST(arg_P1 "abc") + TEST(arg_P2 "UNDEFINED") + endfunction() + test_cmp0174_old_repeated_arg(P1 P1 abc) 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" "") + cmake_parse_arguments(PARSE_ARGV 0 arg "" "P1;P2;P3" "") TEST(arg_KEYWORDS_MISSING_VALUES "P2") TEST(arg_P1 "UNDEFINED") TEST(arg_P2 "UNDEFINED") + TEST(arg_P3 "UNDEFINED") endfunction() test_cmp0174_warn(P1 "" P2) + + function(test_cmp0174_warn_repeated_arg) + cmake_parse_arguments(PARSE_ARGV 0 arg "" "P1;P2" "") + TEST(arg_KEYWORDS_MISSING_VALUES "P1") + TEST(arg_P1 "abc") + TEST(arg_P2 "UNDEFINED") + endfunction() + test_cmp0174_warn_repeated_arg(P1 P1 abc) endblock() |