Merge topic 'add-subparsers'

aaeffdfe6b cmArgumentParser: Refactor to allow for nested parsers
18f818f556 cmArgumentParser: Move parser state into dedicated struct

Acked-by: Kitware Robot <kwrobot@kitware.com>
Tested-by: buildbot <buildbot@kitware.com>
Merge-request: !10963

Source/cmArgumentParser.cxx

@@ -56,8 +56,8 @@ auto PositionActionMap::Find(std::size_t pos) const -> const_iterator
 void Instance::Bind(std::function<Continue(cm::string_view)> 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<std::string>& 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<std::vector<std::string>>& 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 const& 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);
     }
   }
 }

Source/cmArgumentParser.h

@@ -165,16 +165,49 @@ 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<Continue(cm::string_view)> 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<std::string>* unparsedArguments, void* result = nullptr)
-    : Bindings(bindings)
-    , ParseResults(parseResult)
+    : ParseResults(parseResult)
     , UnparsedArguments(unparsedArguments)
-    , Result(result)
   {
+    PushState(bindings, result);
+  }
+
+  ParserState& GetState() { return this->Stack.back(); }
+
+  void PushState(ActionMap const& bindings, void* result)
+  {
+    this->Stack.emplace_back(bindings, result);
+  }
+
+  void PopState()
+  {
+    if (!this->Stack.empty()) {
+      this->Stack.pop_back();
+    }
   }
 
   void Bind(std::function<Continue(cm::string_view)> f, ExpectAtLeast expect);
@@ -185,6 +218,7 @@ public:
   void Bind(MaybeEmpty<std::vector<std::string>>& val);
   void Bind(NonEmpty<std::vector<std::string>>& val);
   void Bind(std::vector<std::vector<std::string>>& val);
+
   template <typename U>
   void Bind(NonEmpty<std::vector<std::pair<std::string, U>>>& val,
             U const& context)
@@ -217,27 +251,43 @@ public:
   }
 
   template <typename Range>
-  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) {
-      this->Consume(pos++, arg);
+      for (size_t j = 0; j < FindKeywordOwnerLevel(arg); ++j) {
+        this->PopState();
+      }
+
+      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:
-  ActionMap const& Bindings;
+  std::vector<ParserState> Stack;
   ParseResult* ParseResults = nullptr;
   std::vector<std::string>* UnparsedArguments = nullptr;
-  void* Result = nullptr;
 
-  cm::string_view Keyword;
-  std::size_t KeywordValuesSeen = 0;
-  std::size_t KeywordValuesExpected = 0;
-  std::function<Continue(cm::string_view)> KeywordValueFunc;
-  bool DoneWithPositional = false;
-
-  void Consume(std::size_t pos, cm::string_view arg);
+  void Consume(cm::string_view arg);
   void FinishKeyword();
 
   template <typename Result>
@@ -250,6 +300,8 @@ template <typename Result>
 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 <typename T, typename cT = cm::member_pointer_class_t<T>,
@@ -259,7 +311,7 @@ public:
   cmArgumentParser& Bind(cm::static_string_view name, T member)
   {
     this->Base::Bind(name, [member](Instance& instance) {
-      instance.Bind(static_cast<Result*>(instance.Result)->*member);
+      instance.Bind(static_cast<Result*>(instance.GetState().Result)->*member);
     });
     return *this;
   }
@@ -269,7 +321,7 @@ public:
                                     T Result::*member, U Result::*context)
   {
     this->Base::Bind(name, [member, context](Instance& instance) {
-      auto* result = static_cast<Result*>(instance.Result);
+      auto* result = static_cast<Result*>(instance.GetState().Result);
       instance.Bind(result->*member, result->*context);
     });
     return *this;
@@ -280,7 +332,7 @@ public:
                          ExpectAtLeast expect = { 1 })
   {
     this->Base::Bind(name, [member, expect](Instance& instance) {
-      Result* result = static_cast<Result*>(instance.Result);
+      Result* result = static_cast<Result*>(instance.GetState().Result);
       instance.Bind(
         [result, member](cm::string_view arg) -> Continue {
           return (result->*member)(arg);
@@ -296,8 +348,8 @@ public:
                          ExpectAtLeast expect = { 1 })
   {
     this->Base::Bind(name, [member, expect](Instance& instance) {
-      Result* result = static_cast<Result*>(instance.Result);
-      cm::string_view keyword = instance.Keyword;
+      Result* result = static_cast<Result*>(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 +364,7 @@ public:
                          ExpectAtLeast expect = { 1 })
   {
     this->Base::Bind(name, [f, expect](Instance& instance) {
-      Result* result = static_cast<Result*>(instance.Result);
+      Result* result = static_cast<Result*>(instance.GetState().Result);
       instance.Bind(
         [result, &f](cm::string_view arg) -> Continue {
           return f(*result, arg);
@@ -328,8 +380,8 @@ public:
     ExpectAtLeast expect = { 1 })
   {
     this->Base::Bind(name, [f, expect](Instance& instance) {
-      Result* result = static_cast<Result*>(instance.Result);
-      cm::string_view keyword = instance.Keyword;
+      Result* result = static_cast<Result*>(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 +397,7 @@ public:
     this->Base::Bind(
       position,
       [member](Instance& instance, std::size_t, cm::string_view arg) {
-        Result* result = static_cast<Result*>(instance.Result);
+        Result* result = static_cast<Result*>(instance.GetState().Result);
         result->*member = arg;
       });
     return *this;
@@ -356,7 +408,8 @@ public:
   {
     this->Base::BindParsedKeyword(
       [member](Instance& instance, cm::string_view arg) {
-        (static_cast<Result*>(instance.Result)->*member).emplace_back(arg);
+        (static_cast<Result*>(instance.GetState().Result)->*member)
+          .emplace_back(arg);
       });
     return *this;
   }
@@ -368,11 +421,44 @@ public:
   cmArgumentParser& BindTrailingArgs(T member)
   {
     this->Base::BindTrailingArgs([member](Instance& instance) {
-      instance.Bind(static_cast<Result*>(instance.Result)->*member);
+      instance.Bind(static_cast<Result*>(instance.GetState().Result)->*member);
     });
     return *this;
   }
 
+  template <typename T, typename U>
+  cmArgumentParser& BindSubParser(cm::static_string_view name,
+                                  cmArgumentParser<T>& parser,
+                                  cm::optional<T> U::*member)
+  {
+
+    this->Base::Bind(name, [name, parser, member](Instance& instance) {
+      auto* parentResult = static_cast<Result*>(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 <typename T, typename U>
+  cmArgumentParser& BindSubParser(cm::static_string_view name,
+                                  cmArgumentParser<T>& parser, T U::*member)
+  {
+    this->Base::Bind(name, [name, parser, member](Instance& instance) {
+      auto* parentResult = static_cast<Result*>(instance.GetState().Result);
+      auto* childResult = &(parentResult->*member);
+      instance.Bind(nullptr, ExpectAtLeast{ 1 });
+      instance.PushState(parser.Bindings, childResult);
+      instance.Consume(name);
+    });
+
+    return *this;
+  }
+
   template <typename Range>
   bool Parse(Result& result, Range const& args,
              std::vector<std::string>* unparsedArguments,
@@ -405,6 +491,8 @@ template <>
 class cmArgumentParser<void> : private ArgumentParser::Base
 {
 public:
+  using Base::Bindings;
+
   template <typename T>
   cmArgumentParser& Bind(cm::static_string_view name, T& ref)
   {
@@ -429,7 +517,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);
@@ -463,6 +551,32 @@ public:
     return *this;
   }
 
+  template <typename T, typename U>
+  cmArgumentParser& BindSubParser(cm::static_string_view name,
+                                  cmArgumentParser<T>& parser,
+                                  cm::optional<U>& 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 <typename T, typename U>
+  cmArgumentParser& BindSubParser(cm::static_string_view name,
+                                  cmArgumentParser<T>& 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 <typename Range>
   ParseResult Parse(Range const& args,
                     std::vector<std::string>* unparsedArguments,

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<std::string> SubCommand;
+    bool SubOption = false;
+    ArgumentParser::NonEmpty<std::string> SubString;
+    ArgumentParser::NonEmpty<std::vector<std::string>> SubList;
+    std::vector<cm::string_view> ParsedKeywords;
+  };
+
   bool Option1 = false;
   bool Option2 = false;
 
@@ -83,6 +91,9 @@ struct Result : public ArgumentParser::ParseResult
                             : ArgumentParser::Continue::No;
   }
 
+  cm::optional<SubResult> Sub;
+  ArgumentParser::NonEmpty<std::string> Parent;
+
   ArgumentParser::Maybe<std::string> UnboundMaybe{ 'u', 'n', 'b', 'o',
                                                    'u', 'n', 'd' };
   ArgumentParser::MaybeEmpty<std::vector<std::string>> UnboundMaybeEmpty{
@@ -131,7 +142,12 @@ std::initializer_list<cm::string_view> 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<cm::string_view> subParsedKeywords = {
+    "SUB_CMD", "SUB_OPTION", "SUB_STRING", "SUB_LIST"
+  };
+
   static std::map<std::string, std::vector<std::string>> 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<std::string>({ "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<std::string> unparsedArguments;
 
   std::function<ArgumentParser::Continue(cm::string_view, cm::string_view)>
@@ -305,6 +338,13 @@ bool testArgumentParserDynamic()
   ASSERT_TRUE(parserDynamic.HasKeyword("OPTION_1"_s));
   ASSERT_TRUE(!parserDynamic.HasKeyword("NOT_AN_OPTION"_s));
 
+  auto subParser = cmArgumentParser<void>{}
+                     .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<ArgumentParser::ParseResult&>(result) =
     cmArgumentParser<void>{}
       .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<resultType::SubResult>{}                                 \
+      .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<resultType>{}                                            \
       .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<std::vector<std::string>> nonEmptyVecStr;
   nonEmptyVecStr = std::vector<std::string>{ "" };
-
   return true;
 }
 
-} // namespace
+}
 
 int testArgumentParser(int /*unused*/, char* /*unused*/[])
 {