[PATCH] D83639: [OptTable] Support grouped short options

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

This looks fine to me probably. 3 last nits.



================
Comment at: llvm/include/llvm/Option/OptTable.h:94
+  /// \return The parsed argument, or 0 if the argument is a missing value.
+  Arg *parseOneArgGrouped(InputArgList &Args, unsigned &Index) const;
+
----------------
Doesn't seem you need doxygen comments for a private method?


================
Comment at: llvm/lib/Option/OptTable.cpp:380
+
+  return new Arg(getOption(TheUnknownOptionID), Str, Index++, CStr);
+}
----------------
++Index


================
Comment at: llvm/lib/Option/Option.cpp:111
+                            unsigned &Index) const {
+  unsigned ArgSize = (unsigned)Spelling.size();
   switch (getKind()) {
----------------
why `unsigned` and not `size_t`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83639/new/

https://reviews.llvm.org/D83639