refactor arg parser

src/arg_parser.cpp

@@ -26,10 +26,12 @@ namespace ydsh {
 // ###############################
 
 RequiredOptionSet::RequiredOptionSet(const std::vector<ArgEntry> &entries) {
-  for (auto &e : entries) {
+  const size_t size = entries.size();
+  for (size_t i = 0; i < size; i++) {
+    auto &e = entries[i];
     if (e.isRequire() || e.isPositional()) {
-      assert(e.getIndexAsInt() <= UINT8_MAX);
-      auto v = static_cast<unsigned char>(e.getIndexAsInt());
+      assert(i <= SYS_LIMIT_ARG_ENTRY_MAX);
+      auto v = static_cast<unsigned short>(i);
       assert(this->values.empty() || this->values.back() < v);
       this->values.push_back(v);
     }
@@ -61,14 +63,13 @@ bool ArgParserObject::parseAll(DSState &state, const ArrayObject &args, BaseObje
       help = true;
       continue;
     }
-    const unsigned int index = toUnderlying(ret.getOpt());
-    assert(index <= UINT8_MAX);
-    requiredSet.del(static_cast<unsigned char>(index));
-    auto &entry = this->instance.getEntries()[index];
-    assert(entry.getIndexAsInt() == index);
+    const auto entryIndex = toUnderlying(ret.getOpt());
+    assert(entryIndex < SYS_LIMIT_ARG_ENTRY_MAX);
+    requiredSet.del(entryIndex);
+    auto &entry = this->instance.getEntries()[entryIndex];
     switch (entry.getParseOp()) {
     case OptParseOp::NO_ARG:
-      out[index] = DSValue::createBool(true); // set flag
+      out[entryIndex] = DSValue::createBool(true); // set flag
       continue;
     case OptParseOp::HAS_ARG:
     case OptParseOp::OPT_ARG:
@@ -102,15 +103,15 @@ bool ArgParserObject::parseAll(DSState &state, const ArrayObject &args, BaseObje
 }
 
 bool ArgParserObject::checkAndSetArg(DSState &state, const ArgEntry &entry, StringRef arg,
-                                     BaseObject &out) {
+                                     BaseObject &out) const {
   std::string err;
   int64_t v = 0;
   if (entry.checkArg(arg, v, err)) {
-    unsigned int index = entry.getIndexAsInt();
+    unsigned int offset = entry.getFieldOffset();
     if (entry.getCheckerKind() == ArgEntry::CheckerKind::INT) {
-      out[index] = DSValue::createInt(v);
+      out[offset] = DSValue::createInt(v);
     } else {
-      out[index] = DSValue::createStr(arg);
+      out[offset] = DSValue::createStr(arg);
     }
     return true;
   } else {
@@ -150,12 +151,12 @@ bool ArgParserObject::checkRequireOrPositionalArgs(DSState &state,
     if (begin != end) {
       StringRef arg = *begin;
       ++begin;
-      if (e.isRemainArg() && out[e.getIndexAsInt()].isInvalid()) {
-        out[e.getIndexAsInt()] =
+      if (e.isRemainArg() && out[e.getFieldOffset()].isInvalid()) {
+        out[e.getFieldOffset()] =
             DSValue::create<ArrayObject>(state.typePool.get(TYPE::StringArray));
       }
       if (e.isRemainArg()) {
-        auto &obj = typeAs<ArrayObject>(out[e.getIndexAsInt()]);
+        auto &obj = typeAs<ArrayObject>(out[e.getFieldOffset()]);
         if (!obj.append(state, DSValue::createStr(arg))) {
           return false;
         }

src/arg_parser.h

@@ -27,7 +27,7 @@ class RequiredOptionSet {
   /**
    * maintains require option index and positional argument index
    */
-  FlexBuffer<unsigned char> values; // must be sorted
+  FlexBuffer<unsigned short> values; // must be sorted
 
 public:
   explicit RequiredOptionSet(const std::vector<ArgEntry> &entries);
@@ -71,7 +71,7 @@ class ArgParserObject : public ObjectWithRtti<ObjectKind::ArgParser> {
    * @return
    * if has error, return false
    */
-  bool checkAndSetArg(DSState &state, const ArgEntry &entry, StringRef arg, BaseObject &out);
+  bool checkAndSetArg(DSState &state, const ArgEntry &entry, StringRef arg, BaseObject &out) const;
 
   bool checkRequireOrPositionalArgs(DSState &state, const RequiredOptionSet &requiredSet,
                                     StrArrayIter &begin, StrArrayIter end, BaseObject &out);

src/arg_parser_base.cpp

@@ -39,13 +39,6 @@ void ArgEntry::destroyCheckerData() {
   }
 }
 
-OptParser<ArgEntry::Index>::Option ArgEntry::toOption() const {
-  assert(this->isOption());
-  const char *arg = this->argName ? this->argName.get() : "arg";
-  const char *d = this->detail ? this->detail.get() : "";
-  return {this->index, this->shortOptName, this->longOptName.get(), this->parseOp, arg, d};
-}
-
 bool ArgEntry::checkArg(StringRef arg, int64_t &out, std::string &err) const {
   out = 0;
   switch (this->checkerKind) {
@@ -102,6 +95,13 @@ bool ArgEntry::checkArg(StringRef arg, int64_t &out, std::string &err) const {
 
 template class OptParser<ArgEntry::Index>; // explicit instantiation
 
+static OptParser<ArgEntry::Index>::Option toOption(const ArgEntry &entry, ArgEntry::Index index) {
+  assert(entry.isOption());
+  const char *arg = entry.getArgName() ? entry.getArgName() : "arg";
+  const char *d = entry.getDetail() ? entry.getDetail() : "";
+  return {index, entry.getShortName(), entry.getLongName(), entry.getParseOp(), arg, d};
+}
+
 ArgParser ArgParser::create(const std::vector<ArgEntry> &entries) {
   size_t size = 1; // reserved for help
   for (auto &e : entries) {
@@ -113,7 +113,9 @@ ArgParser ArgParser::create(const std::vector<ArgEntry> &entries) {
   size_t index = 0;
   for (auto &e : entries) {
     if (e.isOption()) {
-      options[index++] = e.toOption();
+      auto i = static_cast<ArgEntry::Index>(index);
+      options[index] = toOption(e, i);
+      index++;
     }
   }
   options[size - 1] = {ArgEntry::HELP, 'h', "help", OptParseOp::NO_ARG, "show this help message"};

src/arg_parser_base.h

@@ -19,19 +19,31 @@
 
 #include <limits>
 
+#include "constant.h"
 #include "misc/buffer.hpp"
 #include "misc/enum_util.hpp"
 #include "misc/opt_parser.hpp"
 #include "misc/resource.hpp"
 
 namespace ydsh {
 
+enum class ArgEntryAttr : unsigned short {
+  REQUIRE = 1u << 0u,    // require option
+  POSITIONAL = 1u << 1u, // positional argument
+  REMAIN = 1u << 2u,     // remain argument (last positional argument that accept string array)
+};
+
+template <>
+struct allow_enum_bitop<ArgEntryAttr> : std::true_type {};
+
 class ArgEntry {
 public:
   enum class Index : unsigned short {};
 
-  static constexpr auto HELP =
-      static_cast<Index>(std::numeric_limits<std::underlying_type_t<Index>>::max());
+  static constexpr auto HELP = static_cast<Index>(SYS_LIMIT_ARG_ENTRY_MAX);
+
+  static_assert(std::numeric_limits<std::underlying_type_t<Index>>::max() ==
+                SYS_LIMIT_ARG_ENTRY_MAX);
 
   enum class CheckerKind : unsigned char {
     NOP,    // no check
@@ -40,11 +52,9 @@ class ArgEntry {
   };
 
 private:
-  Index index{0}; // for corresponding field offset
+  unsigned char fieldOffset{0}; // corresponding field offset
   OptParseOp parseOp{OptParseOp::NO_ARG};
-  bool require{false};
-  bool positional{false}; // for positional arguments
-  bool remain{false};     // for last positional argument (array)
+  ArgEntryAttr attr{};
   CheckerKind checkerKind{CheckerKind::NOP};
   char shortOptName{0};
   CStrPtr longOptName{nullptr};  // may be null
@@ -64,34 +74,27 @@ class ArgEntry {
   };
 
 public:
-  explicit ArgEntry(unsigned int index)
-      : index(static_cast<Index>(static_cast<std::underlying_type_t<Index>>(index))),
-        intRange({0, 0}) {}
+  explicit ArgEntry(unsigned char fieldOffset) : fieldOffset(fieldOffset), intRange({0, 0}) {}
 
   ~ArgEntry();
 
-  Index getIndex() const { return this->index; }
-
-  unsigned int getIndexAsInt() const { return toUnderlying(this->getIndex()); }
+  unsigned int getFieldOffset() const { return this->fieldOffset; }
 
   void setParseOp(OptParseOp op) { this->parseOp = op; }
 
   OptParseOp getParseOp() const { return this->parseOp; }
 
-  void setRequire(bool r) { this->require = r; }
+  void setAttr(ArgEntryAttr a) { this->attr = a; }
 
-  bool isRequire() const { return this->require; }
+  ArgEntryAttr getAttr() const { return this->attr; }
 
-  void setPositional(bool p) { this->positional = p; }
+  bool hasAttr(ArgEntryAttr a) const { return hasFlag(this->attr, a); }
 
-  bool isPositional() const { return this->positional; }
+  bool isRequire() const { return this->hasAttr(ArgEntryAttr::REQUIRE); }
 
-  void setRemainArg(bool r) {
-    this->setPositional(r);
-    this->remain = r;
-  }
+  bool isPositional() const { return this->hasAttr(ArgEntryAttr::POSITIONAL); }
 
-  bool isRemainArg() const { return this->remain; }
+  bool isRemainArg() const { return this->hasAttr(ArgEntryAttr::REMAIN); }
 
   void setIntRange(int64_t min, int64_t max) {
     this->destroyCheckerData();
@@ -129,9 +132,9 @@ class ArgEntry {
 
   void setDetail(const char *value) { this->detail.reset(strdup(value)); }
 
-  bool isOption() const { return !this->isPositional(); }
+  const char *getDetail() const { return this->detail.get(); }
 
-  OptParser<ArgEntry::Index>::Option toOption() const;
+  bool isOption() const { return !this->isPositional(); }
 
   /**
    *

src/constant.h

@@ -461,6 +461,7 @@ constexpr size_t SYS_LIMIT_KILL_RING_MAX = UINT8_MAX;
 constexpr size_t SYS_LIMIT_DIRSTACK_SIZE = UINT8_MAX;
 constexpr size_t SYS_LIMIT_INPUT_SIZE = INT32_MAX;
 constexpr size_t SYS_LIMIT_HIST_SIZE = UINT16_MAX;
+constexpr size_t SYS_LIMIT_ARG_ENTRY_MAX = UINT16_MAX;
 
 // helper macro definition
 #if 1