From f46b2e914256322fc8d33b425ec01e9d9c1496ba Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 6 Jul 2022 11:45:57 -0400 Subject: cmArgumentParser: Model maybe-missing string with wrapper type Bindings to `std::string` require one value. Some clients have been filtering `keywordsMissingValue` to support keywords that tolerate a missing value. Offer them a type-safe way to achieve this instead. --- Source/cmArgumentParser.cxx | 7 +++++++ Source/cmArgumentParser.h | 1 + Source/cmArgumentParserTypes.h | 5 +++++ Source/cmFileCommand.cxx | 38 +++++++++++++---------------------- Tests/CMakeLib/testArgumentParser.cxx | 5 +++++ 5 files changed, 32 insertions(+), 24 deletions(-) diff --git a/Source/cmArgumentParser.cxx b/Source/cmArgumentParser.cxx index 1bb905104e..f0c4cfcbe0 100644 --- a/Source/cmArgumentParser.cxx +++ b/Source/cmArgumentParser.cxx @@ -46,6 +46,13 @@ void Instance::Bind(std::string& val) this->ExpectValue = true; } +void Instance::Bind(Maybe& val) +{ + this->CurrentString = &val; + this->CurrentList = nullptr; + this->ExpectValue = false; +} + void Instance::Bind(MaybeEmpty>& val) { this->CurrentString = nullptr; diff --git a/Source/cmArgumentParser.h b/Source/cmArgumentParser.h index 2f0a76d8a3..26148d9aa2 100644 --- a/Source/cmArgumentParser.h +++ b/Source/cmArgumentParser.h @@ -39,6 +39,7 @@ public: void Bind(bool& val); void Bind(std::string& val); + void Bind(Maybe& val); void Bind(MaybeEmpty>& val); void Bind(NonEmpty>& val); void Bind(std::vector>& val); diff --git a/Source/cmArgumentParserTypes.h b/Source/cmArgumentParserTypes.h index 7d4ce6fcdc..9afa5c7166 100644 --- a/Source/cmArgumentParserTypes.h +++ b/Source/cmArgumentParserTypes.h @@ -6,6 +6,11 @@ namespace ArgumentParser { +template +struct Maybe : public T +{ +}; + template struct MaybeEmpty : public T { diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index 22ab5f74c8..d2aa63cfaa 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -28,7 +28,6 @@ #include "cm_sys_stat.h" -#include "cmAlgorithms.h" #include "cmArgumentParser.h" #include "cmArgumentParserTypes.h" #include "cmCMakePath.h" @@ -3211,7 +3210,8 @@ bool HandleConfigureCommand(std::vector const& args, cm::optional Content; bool EscapeQuotes = false; bool AtOnly = false; - std::string NewlineStyle; + // "NEWLINE_STYLE" requires one value, but we use a custom check below. + ArgumentParser::Maybe NewlineStyle; }; static auto const parser = @@ -3236,15 +3236,10 @@ bool HandleConfigureCommand(std::vector const& args, return false; } - // Arguments that are allowed to be empty lists. Keep entries sorted! - static const std::vector LIST_ARGS = { - "NEWLINE_STYLE"_s, // Filter here so we can issue a custom error below. - }; - auto kwbegin = keywordsMissingValues.cbegin(); - auto kwend = cmRemoveMatching(keywordsMissingValues, LIST_ARGS); - if (kwend != kwbegin) { - status.SetError(cmStrCat("CONFIGURE keywords missing values:\n ", - cmJoin(cmMakeRange(kwbegin, kwend), "\n "))); + if (!keywordsMissingValues.empty()) { + status.SetError( + cmStrCat("CONFIGURE keywords missing values:\n ", + cmJoin(cmMakeRange(keywordsMissingValues), "\n "))); cmSystemTools::SetFatalErrorOccurred(); return false; } @@ -3347,7 +3342,10 @@ bool HandleArchiveCreateCommand(std::vector const& args, std::string Format; std::string Compression; std::string CompressionLevel; - std::string MTime; + // "MTIME" should require one value, but it has long been accidentally + // accepted without one and treated as if an empty value were given. + // Fixing this would require a policy. + ArgumentParser::Maybe MTime; bool Verbose = false; // "PATHS" requires at least one value, but use a custom check below. ArgumentParser::MaybeEmpty> Paths; @@ -3375,18 +3373,10 @@ bool HandleArchiveCreateCommand(std::vector const& args, return false; } - // Arguments that are allowed to be empty lists. Keep entries sorted! - static const std::vector LIST_ARGS = { - "MTIME"_s, // "MTIME" should not be in this list because it requires one - // value, but it has long been accidentally accepted without - // one and treated as if an empty value were given. - // Fixing this would require a policy. - }; - auto kwbegin = keywordsMissingValues.cbegin(); - auto kwend = cmRemoveMatching(keywordsMissingValues, LIST_ARGS); - if (kwend != kwbegin) { - status.SetError(cmStrCat("Keywords missing values:\n ", - cmJoin(cmMakeRange(kwbegin, kwend), "\n "))); + if (!keywordsMissingValues.empty()) { + status.SetError( + cmStrCat("Keywords missing values:\n ", + cmJoin(cmMakeRange(keywordsMissingValues), "\n "))); cmSystemTools::SetFatalErrorOccurred(); return false; } diff --git a/Tests/CMakeLib/testArgumentParser.cxx b/Tests/CMakeLib/testArgumentParser.cxx index 85650ed026..ecfc5fcfe8 100644 --- a/Tests/CMakeLib/testArgumentParser.cxx +++ b/Tests/CMakeLib/testArgumentParser.cxx @@ -23,6 +23,7 @@ struct Result std::string String1; cm::optional String2; cm::optional String3; + ArgumentParser::Maybe String4; ArgumentParser::NonEmpty> List1; ArgumentParser::NonEmpty> List2; @@ -44,6 +45,7 @@ std::initializer_list const args = { "STRING_1", // string arg missing value "STRING_2", "foo", "bar", // string arg + unparsed value, presence captured // "STRING_3", // string arg that is not present + "STRING_4", // string arg allowed to be missing value "LIST_1", // list arg missing values "LIST_2", "foo", "bar", // list arg with 2 elems "LIST_3", "bar", // list arg ... @@ -83,6 +85,7 @@ bool verifyResult(Result const& result, ASSERT_TRUE(result.String2); ASSERT_TRUE(*result.String2 == "foo"); ASSERT_TRUE(!result.String3); + ASSERT_TRUE(result.String4.empty()); ASSERT_TRUE(result.List1.empty()); ASSERT_TRUE(result.List2 == foobar); @@ -122,6 +125,7 @@ bool testArgumentParserDynamic() .Bind("STRING_1"_s, result.String1) .Bind("STRING_2"_s, result.String2) .Bind("STRING_3"_s, result.String3) + .Bind("STRING_4"_s, result.String4) .Bind("LIST_1"_s, result.List1) .Bind("LIST_2"_s, result.List2) .Bind("LIST_3"_s, result.List3) @@ -146,6 +150,7 @@ bool testArgumentParserStatic() .Bind("STRING_1"_s, &Result::String1) .Bind("STRING_2"_s, &Result::String2) .Bind("STRING_3"_s, &Result::String3) + .Bind("STRING_4"_s, &Result::String4) .Bind("LIST_1"_s, &Result::List1) .Bind("LIST_2"_s, &Result::List2) .Bind("LIST_3"_s, &Result::List3) -- cgit v1.2.1