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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/Option/Option.h:224-225
 private:
-  Arg *acceptInternal(const ArgList &Args, unsigned &Index,
-                      unsigned ArgSize) const;
+  Arg *acceptInternal(const ArgList &Args, StringRef CurArg,
+                      unsigned &Index) const;
 
----------------
jhenderson wrote:
> It might be nice to keep the `Index` parameter in the same place as it was before (i.e. move `CurArg` after it). I don't feel strongly about this though if you prefer your current approach.
I want to keep out parameters in the end.


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83639