From e6d1e29ffa6bd3141a769d1281f3407ed0774139 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 6 Jul 2022 11:30:22 -0400 Subject: cmArgumentParser: Model maybe-empty and non-empty lists with wrapper types Previously bindings to `std::vector` required at least one value. Some clients have been filtering `keywordsMissingValue` to support keywords followed by empty lists. Instead, require clients to specify whether a keyword's list can be empty as part of the binding type. --- Source/CTest/cmCTestCoverageCommand.h | 3 +- Source/CTest/cmCTestSubmitCommand.h | 7 ++-- Source/CTest/cmCTestUploadCommand.h | 3 +- Source/cmArgumentParser.cxx | 11 +++++- Source/cmArgumentParser.h | 5 ++- Source/cmArgumentParserTypes.h | 19 +++++++++ Source/cmCMakeLanguageCommand.cxx | 3 +- Source/cmDefinePropertyCommand.cxx | 5 ++- Source/cmExportCommand.cxx | 3 +- Source/cmFileCommand.cxx | 72 ++++++++++++++--------------------- Source/cmInstallCommand.cxx | 60 +++++++++++++++-------------- Source/cmInstallCommandArguments.h | 5 ++- Source/cmParseArgumentsCommand.cxx | 4 +- Source/cmTargetSourcesCommand.cxx | 5 ++- Tests/CMakeLib/testArgumentParser.cxx | 17 ++++++--- 15 files changed, 128 insertions(+), 94 deletions(-) create mode 100644 Source/cmArgumentParserTypes.h diff --git a/Source/CTest/cmCTestCoverageCommand.h b/Source/CTest/cmCTestCoverageCommand.h index dc8f71e..55c68b2 100644 --- a/Source/CTest/cmCTestCoverageCommand.h +++ b/Source/CTest/cmCTestCoverageCommand.h @@ -11,6 +11,7 @@ #include #include +#include "cmArgumentParserTypes.h" // IWYU pragma: keep #include "cmCTestHandlerCommand.h" #include "cmCommand.h" @@ -44,5 +45,5 @@ protected: void BindArguments() override; cmCTestGenericHandler* InitializeHandler() override; - cm::optional> Labels; + cm::optional>> Labels; }; diff --git a/Source/CTest/cmCTestSubmitCommand.h b/Source/CTest/cmCTestSubmitCommand.h index 903bb64..b67f182 100644 --- a/Source/CTest/cmCTestSubmitCommand.h +++ b/Source/CTest/cmCTestSubmitCommand.h @@ -10,6 +10,7 @@ #include +#include "cmArgumentParserTypes.h" #include "cmCTestHandlerCommand.h" class cmCommand; @@ -50,7 +51,7 @@ protected: std::string RetryDelay; std::string SubmitURL; - cm::optional> Files; - std::vector HttpHeaders; - cm::optional> Parts; + cm::optional>> Files; + ArgumentParser::MaybeEmpty> HttpHeaders; + cm::optional>> Parts; }; diff --git a/Source/CTest/cmCTestUploadCommand.h b/Source/CTest/cmCTestUploadCommand.h index 0dad2bf..a9d1dd2 100644 --- a/Source/CTest/cmCTestUploadCommand.h +++ b/Source/CTest/cmCTestUploadCommand.h @@ -10,6 +10,7 @@ #include +#include "cmArgumentParserTypes.h" #include "cmCTestHandlerCommand.h" #include "cmCommand.h" @@ -45,5 +46,5 @@ protected: void CheckArguments() override; cmCTestGenericHandler* InitializeHandler() override; - std::vector Files; + ArgumentParser::MaybeEmpty> Files; }; diff --git a/Source/cmArgumentParser.cxx b/Source/cmArgumentParser.cxx index 1a05dbc..1bb9051 100644 --- a/Source/cmArgumentParser.cxx +++ b/Source/cmArgumentParser.cxx @@ -4,6 +4,8 @@ #include +#include "cmArgumentParserTypes.h" + namespace ArgumentParser { auto ActionMap::Emplace(cm::string_view name, Action action) @@ -44,7 +46,14 @@ void Instance::Bind(std::string& val) this->ExpectValue = true; } -void Instance::Bind(std::vector& val) +void Instance::Bind(MaybeEmpty>& val) +{ + this->CurrentString = nullptr; + this->CurrentList = &val; + this->ExpectValue = false; +} + +void Instance::Bind(NonEmpty>& val) { this->CurrentString = nullptr; this->CurrentList = &val; diff --git a/Source/cmArgumentParser.h b/Source/cmArgumentParser.h index 8be5a9c..2f0a76d 100644 --- a/Source/cmArgumentParser.h +++ b/Source/cmArgumentParser.h @@ -14,6 +14,8 @@ #include #include +#include "cmArgumentParserTypes.h" // IWYU pragma: keep + namespace ArgumentParser { class Instance; @@ -37,7 +39,8 @@ public: void Bind(bool& val); void Bind(std::string& val); - void Bind(std::vector& val); + void Bind(MaybeEmpty>& val); + void Bind(NonEmpty>& val); void Bind(std::vector>& val); // cm::optional<> records the presence the keyword to which it binds. diff --git a/Source/cmArgumentParserTypes.h b/Source/cmArgumentParserTypes.h new file mode 100644 index 0000000..7d4ce6f --- /dev/null +++ b/Source/cmArgumentParserTypes.h @@ -0,0 +1,19 @@ +/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying + file Copyright.txt or https://cmake.org/licensing for details. */ +#pragma once + +#include "cmConfigure.h" // IWYU pragma: keep + +namespace ArgumentParser { + +template +struct MaybeEmpty : public T +{ +}; + +template +struct NonEmpty : public T +{ +}; + +} // namespace ArgumentParser diff --git a/Source/cmCMakeLanguageCommand.cxx b/Source/cmCMakeLanguageCommand.cxx index 287f45b..76561c3 100644 --- a/Source/cmCMakeLanguageCommand.cxx +++ b/Source/cmCMakeLanguageCommand.cxx @@ -14,6 +14,7 @@ #include #include "cmArgumentParser.h" +#include "cmArgumentParserTypes.h" #include "cmDependencyProvider.h" #include "cmExecutionStatus.h" #include "cmGlobalGenerator.h" @@ -237,7 +238,7 @@ bool cmCMakeLanguageCommandSET_DEPENDENCY_PROVIDER( struct SetProviderArgs { std::string Command; - std::vector Methods; + ArgumentParser::NonEmpty> Methods; }; auto const ArgsParser = diff --git a/Source/cmDefinePropertyCommand.cxx b/Source/cmDefinePropertyCommand.cxx index faefcb8..31ee665 100644 --- a/Source/cmDefinePropertyCommand.cxx +++ b/Source/cmDefinePropertyCommand.cxx @@ -8,6 +8,7 @@ #include #include "cmArgumentParser.h" +#include "cmArgumentParserTypes.h" #include "cmExecutionStatus.h" #include "cmMakefile.h" #include "cmProperty.h" @@ -51,8 +52,8 @@ bool cmDefinePropertyCommand(std::vector const& args, // Parse remaining arguments. bool inherited = false; std::string PropertyName; - std::vector BriefDocs; - std::vector FullDocs; + ArgumentParser::NonEmpty> BriefDocs; + ArgumentParser::NonEmpty> FullDocs; std::string initializeFromVariable; cmArgumentParser parser; diff --git a/Source/cmExportCommand.cxx b/Source/cmExportCommand.cxx index 4071f99..a58f2b7 100644 --- a/Source/cmExportCommand.cxx +++ b/Source/cmExportCommand.cxx @@ -13,6 +13,7 @@ #include "cmsys/RegularExpression.hxx" #include "cmArgumentParser.h" +#include "cmArgumentParserTypes.h" #include "cmExecutionStatus.h" #include "cmExperimental.h" #include "cmExportBuildAndroidMKGenerator.h" @@ -58,7 +59,7 @@ bool cmExportCommand(std::vector const& args, struct Arguments { std::string ExportSetName; - cm::optional> Targets; + cm::optional>> Targets; std::string Namespace; std::string Filename; std::string AndroidMKFile; diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index 45ec343..22ab5f7 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -30,6 +30,7 @@ #include "cmAlgorithms.h" #include "cmArgumentParser.h" +#include "cmArgumentParserTypes.h" #include "cmCMakePath.h" #include "cmCryptoHash.h" #include "cmELF.h" @@ -2505,7 +2506,7 @@ bool HandleGenerateCommand(std::vector const& args, cm::optional NewLineStyle; bool NoSourcePermissions = false; bool UseSourcePermissions = false; - std::vector FilePermissions; + ArgumentParser::NonEmpty> FilePermissions; }; static auto const parser = @@ -3052,17 +3053,18 @@ bool HandleGetRuntimeDependenciesCommand(std::vector const& args, std::string ConflictingDependenciesPrefix; std::string RPathPrefix; std::string BundleExecutable; - std::vector Executables; - std::vector Libraries; - std::vector Directories; - std::vector Modules; - std::vector PreIncludeRegexes; - std::vector PreExcludeRegexes; - std::vector PostIncludeRegexes; - std::vector PostExcludeRegexes; - std::vector PostIncludeFiles; - std::vector PostExcludeFiles; - std::vector PostExcludeFilesStrict; + ArgumentParser::MaybeEmpty> Executables; + ArgumentParser::MaybeEmpty> Libraries; + ArgumentParser::MaybeEmpty> Directories; + ArgumentParser::MaybeEmpty> Modules; + ArgumentParser::MaybeEmpty> PreIncludeRegexes; + ArgumentParser::MaybeEmpty> PreExcludeRegexes; + ArgumentParser::MaybeEmpty> PostIncludeRegexes; + ArgumentParser::MaybeEmpty> PostExcludeRegexes; + ArgumentParser::MaybeEmpty> PostIncludeFiles; + ArgumentParser::MaybeEmpty> PostExcludeFiles; + ArgumentParser::MaybeEmpty> + PostExcludeFilesStrict; }; static auto const parser = @@ -3098,25 +3100,10 @@ bool HandleGetRuntimeDependenciesCommand(std::vector const& args, return false; } - // Arguments that are allowed to be empty lists. Keep entries sorted! - static const std::vector LIST_ARGS = { - "DIRECTORIES"_s, - "EXECUTABLES"_s, - "LIBRARIES"_s, - "MODULES"_s, - "POST_EXCLUDE_FILES"_s, - "POST_EXCLUDE_FILES_STRICT"_s, - "POST_EXCLUDE_REGEXES"_s, - "POST_INCLUDE_FILES"_s, - "POST_INCLUDE_REGEXES"_s, - "PRE_EXCLUDE_REGEXES"_s, - "PRE_INCLUDE_REGEXES"_s, - }; - 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; } @@ -3362,7 +3349,8 @@ bool HandleArchiveCreateCommand(std::vector const& args, std::string CompressionLevel; std::string MTime; bool Verbose = false; - std::vector Paths; + // "PATHS" requires at least one value, but use a custom check below. + ArgumentParser::MaybeEmpty> Paths; }; static auto const parser = @@ -3393,7 +3381,6 @@ bool HandleArchiveCreateCommand(std::vector const& args, // 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. - "PATHS"_s, // "PATHS" is here only so we can issue a custom error below. }; auto kwbegin = keywordsMissingValues.cbegin(); auto kwend = cmRemoveMatching(keywordsMissingValues, LIST_ARGS); @@ -3496,7 +3483,7 @@ bool HandleArchiveExtractCommand(std::vector const& args, bool Verbose = false; bool ListOnly = false; std::string Destination; - std::vector Patterns; + ArgumentParser::MaybeEmpty> Patterns; bool Touch = false; }; @@ -3520,13 +3507,10 @@ bool HandleArchiveExtractCommand(std::vector const& args, return false; } - // Arguments that are allowed to be empty lists. Keep entries sorted! - static const std::vector LIST_ARGS = { "PATTERNS"_s }; - 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; } @@ -3620,9 +3604,9 @@ bool HandleChmodCommandImpl(std::vector const& args, bool recurse, struct Arguments { - std::vector Permissions; - std::vector FilePermissions; - std::vector DirectoryPermissions; + ArgumentParser::NonEmpty> Permissions; + ArgumentParser::NonEmpty> FilePermissions; + ArgumentParser::NonEmpty> DirectoryPermissions; }; static auto const parser = diff --git a/Source/cmInstallCommand.cxx b/Source/cmInstallCommand.cxx index 35ddf34..b1044a8 100644 --- a/Source/cmInstallCommand.cxx +++ b/Source/cmInstallCommand.cxx @@ -19,6 +19,7 @@ #include "cmsys/Glob.hxx" #include "cmArgumentParser.h" +#include "cmArgumentParserTypes.h" #include "cmExecutionStatus.h" #include "cmExperimental.h" #include "cmExportSet.h" @@ -57,13 +58,13 @@ namespace { struct RuntimeDependenciesArgs { - std::vector Directories; - std::vector PreIncludeRegexes; - std::vector PreExcludeRegexes; - std::vector PostIncludeRegexes; - std::vector PostExcludeRegexes; - std::vector PostIncludeFiles; - std::vector PostExcludeFiles; + ArgumentParser::MaybeEmpty> Directories; + ArgumentParser::MaybeEmpty> PreIncludeRegexes; + ArgumentParser::MaybeEmpty> PreExcludeRegexes; + ArgumentParser::MaybeEmpty> PostIncludeRegexes; + ArgumentParser::MaybeEmpty> PostExcludeRegexes; + ArgumentParser::MaybeEmpty> PostIncludeFiles; + ArgumentParser::MaybeEmpty> PostExcludeFiles; }; auto const RuntimeDependenciesArgHelper = @@ -406,18 +407,18 @@ bool HandleTargetsMode(std::vector const& args, struct ArgVectors { - std::vector Archive; - std::vector Library; - std::vector Runtime; - std::vector Object; - std::vector Framework; - std::vector Bundle; - std::vector Includes; - std::vector PrivateHeader; - std::vector PublicHeader; - std::vector Resource; + ArgumentParser::MaybeEmpty> Archive; + ArgumentParser::MaybeEmpty> Library; + ArgumentParser::MaybeEmpty> Runtime; + ArgumentParser::MaybeEmpty> Object; + ArgumentParser::MaybeEmpty> Framework; + ArgumentParser::MaybeEmpty> Bundle; + ArgumentParser::MaybeEmpty> Includes; + ArgumentParser::MaybeEmpty> PrivateHeader; + ArgumentParser::MaybeEmpty> PublicHeader; + ArgumentParser::MaybeEmpty> Resource; + ArgumentParser::MaybeEmpty> CxxModulesBmi; std::vector> FileSets; - std::vector CxxModulesBmi; }; static auto const argHelper = @@ -441,9 +442,10 @@ bool HandleTargetsMode(std::vector const& args, // now parse the generic args (i.e. the ones not specialized on LIBRARY/ // ARCHIVE, RUNTIME etc. (see above) // These generic args also contain the targets and the export stuff - std::vector targetList; + ArgumentParser::MaybeEmpty> targetList; std::string exports; - cm::optional> runtimeDependenciesArgVector; + cm::optional>> + runtimeDependenciesArgVector; std::string runtimeDependencySetArg; std::vector unknownArgs; cmInstallCommandArguments genericArgs(helper.DefaultComponentName); @@ -1251,10 +1253,10 @@ bool HandleImportedRuntimeArtifactsMode(std::vector const& args, struct ArgVectors { - std::vector Library; - std::vector Runtime; - std::vector Framework; - std::vector Bundle; + ArgumentParser::MaybeEmpty> Library; + ArgumentParser::MaybeEmpty> Runtime; + ArgumentParser::MaybeEmpty> Framework; + ArgumentParser::MaybeEmpty> Bundle; }; static auto const argHelper = cmArgumentParser{} @@ -1268,7 +1270,7 @@ bool HandleImportedRuntimeArtifactsMode(std::vector const& args, // now parse the generic args (i.e. the ones not specialized on LIBRARY, // RUNTIME etc. (see above) - std::vector targetList; + ArgumentParser::MaybeEmpty> targetList; std::string runtimeDependencySetArg; std::vector unknownArgs; cmInstallCommandArguments genericArgs(helper.DefaultComponentName); @@ -1509,7 +1511,7 @@ bool HandleFilesMode(std::vector const& args, // This is the FILES mode. bool programs = (args[0] == "PROGRAMS"); cmInstallCommandArguments ica(helper.DefaultComponentName); - std::vector files; + ArgumentParser::MaybeEmpty> files; ica.Bind(programs ? "PROGRAMS"_s : "FILES"_s, files); std::vector unknownArgs; ica.Parse(args, &unknownArgs); @@ -2140,9 +2142,9 @@ bool HandleRuntimeDependencySetMode(std::vector const& args, struct ArgVectors { - std::vector Library; - std::vector Runtime; - std::vector Framework; + ArgumentParser::MaybeEmpty> Library; + ArgumentParser::MaybeEmpty> Runtime; + ArgumentParser::MaybeEmpty> Framework; }; static auto const argHelper = cmArgumentParser{} diff --git a/Source/cmInstallCommandArguments.h b/Source/cmInstallCommandArguments.h index 79bd945..6e46aac 100644 --- a/Source/cmInstallCommandArguments.h +++ b/Source/cmInstallCommandArguments.h @@ -8,6 +8,7 @@ #include #include "cmArgumentParser.h" +#include "cmArgumentParserTypes.h" class cmInstallCommandArguments : public cmArgumentParser { @@ -44,8 +45,8 @@ private: std::string NamelinkComponent; bool ExcludeFromAll = false; std::string Rename; - std::vector Permissions; - std::vector Configurations; + ArgumentParser::MaybeEmpty> Permissions; + ArgumentParser::MaybeEmpty> Configurations; bool Optional = false; bool NamelinkOnly = false; bool NamelinkSkip = false; diff --git a/Source/cmParseArgumentsCommand.cxx b/Source/cmParseArgumentsCommand.cxx index 5ea35e8..31538b6 100644 --- a/Source/cmParseArgumentsCommand.cxx +++ b/Source/cmParseArgumentsCommand.cxx @@ -10,6 +10,7 @@ #include #include "cmArgumentParser.h" +#include "cmArgumentParserTypes.h" #include "cmExecutionStatus.h" #include "cmMakefile.h" #include "cmMessageType.h" @@ -41,7 +42,8 @@ namespace { using options_map = std::map; using single_map = std::map; -using multi_map = std::map>; +using multi_map = + std::map>>; using options_set = std::set; struct UserArgumentParser : public cmArgumentParser diff --git a/Source/cmTargetSourcesCommand.cxx b/Source/cmTargetSourcesCommand.cxx index 74945fa..e2b0213 100644 --- a/Source/cmTargetSourcesCommand.cxx +++ b/Source/cmTargetSourcesCommand.cxx @@ -9,6 +9,7 @@ #include #include "cmArgumentParser.h" +#include "cmArgumentParserTypes.h" #include "cmExperimental.h" #include "cmFileSet.h" #include "cmGeneratorExpression.h" @@ -28,8 +29,8 @@ struct FileSetArgs { std::string Type; std::string FileSet; - std::vector BaseDirs; - std::vector Files; + ArgumentParser::MaybeEmpty> BaseDirs; + ArgumentParser::MaybeEmpty> Files; }; auto const FileSetArgsParser = cmArgumentParser() diff --git a/Tests/CMakeLib/testArgumentParser.cxx b/Tests/CMakeLib/testArgumentParser.cxx index 52c5861..85650ed 100644 --- a/Tests/CMakeLib/testArgumentParser.cxx +++ b/Tests/CMakeLib/testArgumentParser.cxx @@ -11,6 +11,7 @@ #include #include "cmArgumentParser.h" +#include "cmArgumentParserTypes.h" namespace { @@ -23,11 +24,12 @@ struct Result cm::optional String2; cm::optional String3; - std::vector List1; - std::vector List2; - cm::optional> List3; - cm::optional> List4; - cm::optional> List5; + ArgumentParser::NonEmpty> List1; + ArgumentParser::NonEmpty> List2; + cm::optional>> List3; + cm::optional>> List4; + cm::optional>> List5; + cm::optional>> List6; std::vector> Multi1; std::vector> Multi2; @@ -48,6 +50,7 @@ std::initializer_list const args = { "LIST_3", "foo", // ... with continuation "LIST_4", // list arg missing values, presence captured // "LIST_5", // list arg that is not present + "LIST_6", // list arg allowed to be empty "MULTI_2", // multi list with 0 lists "MULTI_3", "foo", "bar", // multi list with first list with two elems "MULTI_3", "bar", "foo", // multi list with second list with two elems @@ -88,6 +91,8 @@ bool verifyResult(Result const& result, ASSERT_TRUE(result.List4); ASSERT_TRUE(result.List4->empty()); ASSERT_TRUE(!result.List5); + ASSERT_TRUE(result.List6); + ASSERT_TRUE(result.List6->empty()); ASSERT_TRUE(result.Multi1.empty()); ASSERT_TRUE(result.Multi2.size() == 1); @@ -122,6 +127,7 @@ bool testArgumentParserDynamic() .Bind("LIST_3"_s, result.List3) .Bind("LIST_4"_s, result.List4) .Bind("LIST_5"_s, result.List5) + .Bind("LIST_6"_s, result.List6) .Bind("MULTI_1"_s, result.Multi1) .Bind("MULTI_2"_s, result.Multi2) .Bind("MULTI_3"_s, result.Multi3) @@ -145,6 +151,7 @@ bool testArgumentParserStatic() .Bind("LIST_3"_s, &Result::List3) .Bind("LIST_4"_s, &Result::List4) .Bind("LIST_5"_s, &Result::List5) + .Bind("LIST_6"_s, &Result::List6) .Bind("MULTI_1"_s, &Result::Multi1) .Bind("MULTI_2"_s, &Result::Multi2) .Bind("MULTI_3"_s, &Result::Multi3) -- cgit v0.12