From 18f818f556de9a899694fcde4c9c8b2ea4679359 Mon Sep 17 00:00:00 2001 From: Taylor Sasser Date: Mon, 21 Jul 2025 19:48:16 -0400 Subject: [PATCH 1/2] cmArgumentParser: Move parser state into dedicated struct --- Source/cmArgumentParser.cxx | 64 +++++++++++++++++---------------- Source/cmArgumentParser.h | 70 ++++++++++++++++++++++++------------- 2 files changed, 79 insertions(+), 55 deletions(-) diff --git a/Source/cmArgumentParser.cxx b/Source/cmArgumentParser.cxx index 104436be87..ea9261134e 100644 --- a/Source/cmArgumentParser.cxx +++ b/Source/cmArgumentParser.cxx @@ -56,8 +56,8 @@ auto PositionActionMap::Find(std::size_t pos) const -> const_iterator void Instance::Bind(std::function f, ExpectAtLeast expect) { - this->KeywordValueFunc = std::move(f); - this->KeywordValuesExpected = expect.Count; + this->GetState().KeywordValueFunc = std::move(f); + this->GetState().KeywordValuesExpected = expect.Count; } void Instance::Bind(bool& val) @@ -81,7 +81,7 @@ void Instance::Bind(NonEmpty& val) this->Bind( [this, &val](cm::string_view arg) -> Continue { if (arg.empty() && this->ParseResults) { - this->ParseResults->AddKeywordError(this->Keyword, + this->ParseResults->AddKeywordError(this->GetState().Keyword, " empty string not allowed\n"); } val = std::string(arg); @@ -132,48 +132,50 @@ void Instance::Bind(std::vector>& multiVal) ExpectAtLeast{ 0 }); } -void Instance::Consume(std::size_t pos, cm::string_view arg) +void Instance::Consume(cm::string_view arg) { - auto const it = this->Bindings.Keywords.Find(arg); - if (it != this->Bindings.Keywords.end()) { + ParserState& state = this->GetState(); + + auto const it = state.Bindings.Keywords.Find(arg); + if (it != state.Bindings.Keywords.end()) { this->FinishKeyword(); - this->Keyword = it->first; - this->KeywordValuesSeen = 0; - this->DoneWithPositional = true; - if (this->Bindings.ParsedKeyword) { - this->Bindings.ParsedKeyword(*this, it->first); + state.Keyword = it->first; + state.KeywordValuesSeen = 0; + state.DoneWithPositional = true; + if (state.Bindings.ParsedKeyword) { + state.Bindings.ParsedKeyword(*this, it->first); } it->second(*this); return; } - if (!this->DoneWithPositional) { - auto const pit = this->Bindings.Positions.Find(pos); - if (pit != this->Bindings.Positions.end()) { - pit->second(*this, pos, arg); + if (!state.DoneWithPositional) { + auto const pit = state.Bindings.Positions.Find(state.Pos); + if (pit != state.Bindings.Positions.end()) { + pit->second(*this, state.Pos, arg); return; } - if (this->Bindings.TrailingArgs) { - this->Keyword = ""_s; - this->KeywordValuesSeen = 0; - this->DoneWithPositional = true; - this->Bindings.TrailingArgs(*this); - if (!this->KeywordValueFunc) { + if (state.Bindings.TrailingArgs) { + state.Keyword = ""_s; + state.KeywordValuesSeen = 0; + state.DoneWithPositional = true; + state.Bindings.TrailingArgs(*this); + if (!state.KeywordValueFunc) { return; } } } - if (this->KeywordValueFunc) { - switch (this->KeywordValueFunc(arg)) { + if (state.KeywordValueFunc) { + switch (state.KeywordValueFunc(arg)) { case Continue::Yes: break; case Continue::No: - this->KeywordValueFunc = nullptr; + state.KeywordValueFunc = nullptr; break; } - ++this->KeywordValuesSeen; + ++state.KeywordValuesSeen; return; } @@ -184,16 +186,18 @@ void Instance::Consume(std::size_t pos, cm::string_view arg) void Instance::FinishKeyword() { - if (!this->DoneWithPositional) { + ParserState& state = this->GetState(); + + if (state.DoneWithPositional) { return; } - if (this->KeywordValuesSeen < this->KeywordValuesExpected) { + if (state.KeywordValuesSeen < state.KeywordValuesExpected) { if (this->ParseResults) { - this->ParseResults->AddKeywordError(this->Keyword, + this->ParseResults->AddKeywordError(state.Keyword, " missing required value\n"); } - if (this->Bindings.KeywordMissingValue) { - this->Bindings.KeywordMissingValue(*this, this->Keyword); + if (state.Bindings.KeywordMissingValue) { + state.Bindings.KeywordMissingValue(*this, state.Keyword); } } } diff --git a/Source/cmArgumentParser.h b/Source/cmArgumentParser.h index 223c3ce699..69a3377fe7 100644 --- a/Source/cmArgumentParser.h +++ b/Source/cmArgumentParser.h @@ -165,18 +165,40 @@ public: } }; +class ParserState +{ +public: + cm::string_view Keyword; + std::size_t Pos = 0; + std::size_t KeywordValuesSeen = 0; + std::size_t KeywordValuesExpected = 0; + std::function KeywordValueFunc = nullptr; + + ActionMap const& Bindings; + void* Result = nullptr; + bool DoneWithPositional = false; + + ParserState(ActionMap const& bindings, void* result) + : Bindings(bindings) + , Result(result) + { + } +}; + class Instance { public: Instance(ActionMap const& bindings, ParseResult* parseResult, std::vector* unparsedArguments, void* result = nullptr) - : Bindings(bindings) + + : ParseState(bindings, result) , ParseResults(parseResult) , UnparsedArguments(unparsedArguments) - , Result(result) { } + ParserState& GetState() { return ParseState; } + void Bind(std::function f, ExpectAtLeast expect); void Bind(bool& val); void Bind(std::string& val); @@ -219,25 +241,22 @@ public: template void Parse(Range const& args, std::size_t pos = 0) { - for (cm::string_view arg : args) { - this->Consume(pos++, arg); + GetState().Pos = pos; + + for (cm::string_view const arg : args) { + this->Consume(arg); + GetState().Pos++; } + this->FinishKeyword(); } private: - ActionMap const& Bindings; + ParserState ParseState; ParseResult* ParseResults = nullptr; std::vector* UnparsedArguments = nullptr; - void* Result = nullptr; - - cm::string_view Keyword; - std::size_t KeywordValuesSeen = 0; - std::size_t KeywordValuesExpected = 0; - std::function KeywordValueFunc; - bool DoneWithPositional = false; - void Consume(std::size_t pos, cm::string_view arg); + void Consume(cm::string_view arg); void FinishKeyword(); template @@ -259,7 +278,7 @@ public: cmArgumentParser& Bind(cm::static_string_view name, T member) { this->Base::Bind(name, [member](Instance& instance) { - instance.Bind(static_cast(instance.Result)->*member); + instance.Bind(static_cast(instance.GetState().Result)->*member); }); return *this; } @@ -269,7 +288,7 @@ public: T Result::*member, U Result::*context) { this->Base::Bind(name, [member, context](Instance& instance) { - auto* result = static_cast(instance.Result); + auto* result = static_cast(instance.GetState().Result); instance.Bind(result->*member, result->*context); }); return *this; @@ -280,7 +299,7 @@ public: ExpectAtLeast expect = { 1 }) { this->Base::Bind(name, [member, expect](Instance& instance) { - Result* result = static_cast(instance.Result); + Result* result = static_cast(instance.GetState().Result); instance.Bind( [result, member](cm::string_view arg) -> Continue { return (result->*member)(arg); @@ -296,8 +315,8 @@ public: ExpectAtLeast expect = { 1 }) { this->Base::Bind(name, [member, expect](Instance& instance) { - Result* result = static_cast(instance.Result); - cm::string_view keyword = instance.Keyword; + Result* result = static_cast(instance.GetState().Result); + cm::string_view keyword = instance.GetState().Keyword; instance.Bind( [result, member, keyword](cm::string_view arg) -> Continue { return (result->*member)(keyword, arg); @@ -312,7 +331,7 @@ public: ExpectAtLeast expect = { 1 }) { this->Base::Bind(name, [f, expect](Instance& instance) { - Result* result = static_cast(instance.Result); + Result* result = static_cast(instance.GetState().Result); instance.Bind( [result, &f](cm::string_view arg) -> Continue { return f(*result, arg); @@ -328,8 +347,8 @@ public: ExpectAtLeast expect = { 1 }) { this->Base::Bind(name, [f, expect](Instance& instance) { - Result* result = static_cast(instance.Result); - cm::string_view keyword = instance.Keyword; + Result* result = static_cast(instance.GetState().Result); + cm::string_view keyword = instance.GetState().Keyword; instance.Bind( [result, keyword, &f](cm::string_view arg) -> Continue { return f(*result, keyword, arg); @@ -345,7 +364,7 @@ public: this->Base::Bind( position, [member](Instance& instance, std::size_t, cm::string_view arg) { - Result* result = static_cast(instance.Result); + Result* result = static_cast(instance.GetState().Result); result->*member = arg; }); return *this; @@ -356,7 +375,8 @@ public: { this->Base::BindParsedKeyword( [member](Instance& instance, cm::string_view arg) { - (static_cast(instance.Result)->*member).emplace_back(arg); + (static_cast(instance.GetState().Result)->*member) + .emplace_back(arg); }); return *this; } @@ -368,7 +388,7 @@ public: cmArgumentParser& BindTrailingArgs(T member) { this->Base::BindTrailingArgs([member](Instance& instance) { - instance.Bind(static_cast(instance.Result)->*member); + instance.Bind(static_cast(instance.GetState().Result)->*member); }); return *this; } @@ -429,7 +449,7 @@ public: ExpectAtLeast expect = { 1 }) { this->Base::Bind(name, [f, expect](Instance& instance) { - cm::string_view keyword = instance.Keyword; + cm::string_view keyword = instance.GetState().Keyword; instance.Bind( [keyword, &f](cm::string_view arg) -> Continue { return f(keyword, arg); From aaeffdfe6b2310b573db6a57c537322c7dd70512 Mon Sep 17 00:00:00 2001 From: Taylor Sasser Date: Mon, 21 Jul 2025 19:57:41 -0400 Subject: [PATCH 2/2] cmArgumentParser: Refactor to allow for nested parsers --- Source/cmArgumentParser.cxx | 6 +- Source/cmArgumentParser.h | 108 ++++++++++++++++++++++++-- Tests/CMakeLib/testArgumentParser.cxx | 67 ++++++++++++++-- 3 files changed, 164 insertions(+), 17 deletions(-) diff --git a/Source/cmArgumentParser.cxx b/Source/cmArgumentParser.cxx index ea9261134e..2ea2e6a8cf 100644 --- a/Source/cmArgumentParser.cxx +++ b/Source/cmArgumentParser.cxx @@ -186,11 +186,11 @@ void Instance::Consume(cm::string_view arg) void Instance::FinishKeyword() { - ParserState& state = this->GetState(); - - if (state.DoneWithPositional) { + ParserState const& state = this->GetState(); + if (!state.DoneWithPositional) { return; } + if (state.KeywordValuesSeen < state.KeywordValuesExpected) { if (this->ParseResults) { this->ParseResults->AddKeywordError(state.Keyword, diff --git a/Source/cmArgumentParser.h b/Source/cmArgumentParser.h index 69a3377fe7..7893e2e2d9 100644 --- a/Source/cmArgumentParser.h +++ b/Source/cmArgumentParser.h @@ -190,14 +190,25 @@ class Instance public: Instance(ActionMap const& bindings, ParseResult* parseResult, std::vector* unparsedArguments, void* result = nullptr) - - : ParseState(bindings, result) - , ParseResults(parseResult) + : ParseResults(parseResult) , UnparsedArguments(unparsedArguments) { + PushState(bindings, result); + } + + ParserState& GetState() { return this->Stack.back(); } + + void PushState(ActionMap const& bindings, void* result) + { + this->Stack.emplace_back(bindings, result); } - ParserState& GetState() { return ParseState; } + void PopState() + { + if (!this->Stack.empty()) { + this->Stack.pop_back(); + } + } void Bind(std::function f, ExpectAtLeast expect); void Bind(bool& val); @@ -207,6 +218,7 @@ public: void Bind(MaybeEmpty>& val); void Bind(NonEmpty>& val); void Bind(std::vector>& val); + template void Bind(NonEmpty>>& val, U const& context) @@ -239,20 +251,39 @@ public: } template - void Parse(Range const& args, std::size_t pos = 0) + void Parse(Range& args, std::size_t const pos = 0) { GetState().Pos = pos; + for (cm::string_view arg : args) { + for (size_t j = 0; j < FindKeywordOwnerLevel(arg); ++j) { + this->PopState(); + } - for (cm::string_view const arg : args) { this->Consume(arg); GetState().Pos++; } this->FinishKeyword(); + + while (this->Stack.size() > 1) { + this->FinishKeyword(); + this->PopState(); + } + } + + std::size_t FindKeywordOwnerLevel(cm::string_view arg) const + { + for (std::size_t i = Stack.size(); i--;) { + if (this->Stack[i].Bindings.Keywords.Find(arg) != + this->Stack[i].Bindings.Keywords.end()) { + return (this->Stack.size() - 1) - i; + } + } + return 0; } private: - ParserState ParseState; + std::vector Stack; ParseResult* ParseResults = nullptr; std::vector* UnparsedArguments = nullptr; @@ -269,6 +300,8 @@ template class cmArgumentParser : private ArgumentParser::Base { public: + using Base::Bindings; + // I *think* this function could be made `constexpr` when the code is // compiled as C++20. This would allow building a parser at compile time. template , @@ -393,6 +426,39 @@ public: return *this; } + template + cmArgumentParser& BindSubParser(cm::static_string_view name, + cmArgumentParser& parser, + cm::optional U::*member) + { + + this->Base::Bind(name, [name, parser, member](Instance& instance) { + auto* parentResult = static_cast(instance.GetState().Result); + auto& childResult = parentResult->*member; + childResult.emplace(T()); + instance.Bind(nullptr, ExpectAtLeast{ 0 }); + instance.PushState(parser.Bindings, &(*childResult)); + instance.Consume(name); + }); + + return *this; + } + + template + cmArgumentParser& BindSubParser(cm::static_string_view name, + cmArgumentParser& parser, T U::*member) + { + this->Base::Bind(name, [name, parser, member](Instance& instance) { + auto* parentResult = static_cast(instance.GetState().Result); + auto* childResult = &(parentResult->*member); + instance.Bind(nullptr, ExpectAtLeast{ 1 }); + instance.PushState(parser.Bindings, childResult); + instance.Consume(name); + }); + + return *this; + } + template bool Parse(Result& result, Range const& args, std::vector* unparsedArguments, @@ -425,6 +491,8 @@ template <> class cmArgumentParser : private ArgumentParser::Base { public: + using Base::Bindings; + template cmArgumentParser& Bind(cm::static_string_view name, T& ref) { @@ -483,6 +551,32 @@ public: return *this; } + template + cmArgumentParser& BindSubParser(cm::static_string_view name, + cmArgumentParser& parser, + cm::optional& subResult) + { + this->Base::Bind(name, [name, parser, &subResult](Instance& instance) { + subResult.emplace(U()); + instance.Bind(nullptr, ExpectAtLeast{ 0 }); + instance.PushState(parser.Bindings, &(*subResult)); + instance.Consume(name); + }); + return *this; + } + + template + cmArgumentParser& BindSubParser(cm::static_string_view name, + cmArgumentParser& parser, U& subResult) + { + this->Base::Bind(name, [name, parser, &subResult](Instance& instance) { + instance.Bind(nullptr, ExpectAtLeast{ 1 }); + instance.PushState(parser.Bindings, &subResult); + instance.Consume(name); + }); + return *this; + } + template ParseResult Parse(Range const& args, std::vector* unparsedArguments, diff --git a/Tests/CMakeLib/testArgumentParser.cxx b/Tests/CMakeLib/testArgumentParser.cxx index 4bf6a40ca6..789ab12bd6 100644 --- a/Tests/CMakeLib/testArgumentParser.cxx +++ b/Tests/CMakeLib/testArgumentParser.cxx @@ -16,9 +16,17 @@ #include "testCommon.h" namespace { - -struct Result : public ArgumentParser::ParseResult +struct Result : ArgumentParser::ParseResult { + struct SubResult : ParseResult + { + ArgumentParser::NonEmpty SubCommand; + bool SubOption = false; + ArgumentParser::NonEmpty SubString; + ArgumentParser::NonEmpty> SubList; + std::vector ParsedKeywords; + }; + bool Option1 = false; bool Option2 = false; @@ -83,6 +91,9 @@ struct Result : public ArgumentParser::ParseResult : ArgumentParser::Continue::No; } + cm::optional Sub; + ArgumentParser::NonEmpty Parent; + ArgumentParser::Maybe UnboundMaybe{ 'u', 'n', 'b', 'o', 'u', 'n', 'd' }; ArgumentParser::MaybeEmpty> UnboundMaybeEmpty{ @@ -131,7 +142,12 @@ std::initializer_list const args = { "FUNC_3", "foo", "bar", // callback with list arg ... "FUNC_4a", "foo", "ign4", // callback with keyword-dependent arg count "FUNC_4b", "bar", "zot", // callback with keyword-dependent arg count - /* clang-format on */ + "SUB_CMD", "foo", // switch to subparser and set Sub::SUB_CMD to foo + "SUB_OPTION", // subparser option + "SUB_STRING", "sub_value", // subparser string + "SUB_LIST", "a", "b", "c", // subparser list + // Return to main parser (simulate another main option if needed) + "PARENT", "value", }; struct ResultTrailingPos : public ArgumentParser::ParseResult @@ -189,8 +205,15 @@ bool verifyResult(Result const& result, "FUNC_3", "FUNC_4a", "FUNC_4b", + "SUB_CMD", + "PARENT", /* clang-format on */ }; + + static std::vector subParsedKeywords = { + "SUB_CMD", "SUB_OPTION", "SUB_STRING", "SUB_LIST" + }; + static std::map> const func2map = { { "FUNC_2a", { "foo" } }, { "FUNC_2b", { "bar", "zot" } } }; @@ -257,6 +280,14 @@ bool verifyResult(Result const& result, ASSERT_TRUE(result.UnboundNonEmpty == unbound); ASSERT_TRUE(result.UnboundNonEmptyStr == "unbound"); + ASSERT_TRUE(result.Sub->SubCommand == "foo"); + ASSERT_TRUE(result.Sub->SubOption); + ASSERT_TRUE(result.Sub->SubString == "sub_value"); + ASSERT_TRUE(result.Sub->SubList == + std::vector({ "a", "b", "c" })); + ASSERT_TRUE(result.Parent == "value"); + + ASSERT_TRUE(result.Sub->ParsedKeywords == subParsedKeywords); ASSERT_TRUE(result.ParsedKeywords == parsedKeywords); ASSERT_TRUE(result.GetKeywordErrors().size() == keywordErrors.size()); @@ -292,6 +323,8 @@ bool verifyResult(ResultTrailingPos const& result, bool testArgumentParserDynamic() { Result result; + result.Sub = Result::SubResult(); + std::vector unparsedArguments; std::function @@ -305,6 +338,13 @@ bool testArgumentParserDynamic() ASSERT_TRUE(parserDynamic.HasKeyword("OPTION_1"_s)); ASSERT_TRUE(!parserDynamic.HasKeyword("NOT_AN_OPTION"_s)); + auto subParser = cmArgumentParser{} + .Bind("SUB_CMD"_s, result.Sub->SubCommand) + .Bind("SUB_OPTION"_s, result.Sub->SubOption) + .Bind("SUB_STRING"_s, result.Sub->SubString) + .Bind("SUB_LIST"_s, result.Sub->SubList) + .BindParsedKeywords(result.Sub->ParsedKeywords); + static_cast(result) = cmArgumentParser{} .Bind(0, result.Pos0) @@ -349,6 +389,8 @@ bool testArgumentParserDynamic() }) .Bind("FUNC_4a"_s, func4) .Bind("FUNC_4b"_s, func4) + .BindSubParser("SUB_CMD"_s, subParser, result.Sub) + .Bind("PARENT"_s, result.Parent) .BindParsedKeywords(result.ParsedKeywords) .Parse(args, &unparsedArguments); @@ -378,7 +420,17 @@ static auto const parserStaticFunc4 = }; #define BIND_ALL(name, resultType) \ - static auto const name = \ + /* Define the sub-parser first */ \ + static auto sub##name = \ + cmArgumentParser{} \ + .Bind("SUB_CMD"_s, &resultType::SubResult::SubCommand) \ + .Bind("SUB_OPTION"_s, &resultType::SubResult::SubOption) \ + .Bind("SUB_STRING"_s, &resultType::SubResult::SubString) \ + .Bind("SUB_LIST"_s, &resultType::SubResult::SubList) \ + .BindParsedKeywords(&resultType::SubResult::ParsedKeywords); \ + \ + /* Define the main parser, which uses the sub-parser */ \ + static const auto name = \ cmArgumentParser{} \ .Bind(0, &resultType::Pos0) \ .Bind(1, &resultType::Pos1) \ @@ -411,7 +463,9 @@ static auto const parserStaticFunc4 = -> ArgumentParser::Continue { return result.Func3(arg); }) \ .Bind("FUNC_4a"_s, parserStaticFunc4) \ .Bind("FUNC_4b"_s, parserStaticFunc4) \ - .BindParsedKeywords(&resultType::ParsedKeywords) + .BindSubParser("SUB_CMD"_s, sub##name, &resultType::Sub) \ + .Bind("PARENT"_s, &resultType::Parent) \ + .BindParsedKeywords(&resultType::ParsedKeywords); BIND_ALL(parserStatic, Result); BIND_ALL(parserDerivedStatic, Derived); @@ -484,11 +538,10 @@ bool testArgumentParserTypes() ArgumentParser::NonEmpty> nonEmptyVecStr; nonEmptyVecStr = std::vector{ "" }; - return true; } -} // namespace +} int testArgumentParser(int /*unused*/, char* /*unused*/[]) {