[PATCH] D71080: [NFC] Separate getLastArgIntValue to Basic

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.


================
Comment at: clang/lib/Basic/OptionUtils.cpp:18
+template <typename IntTy>
+static IntTy getLastArgIntValueImpl(const ArgList &Args, OptSpecifier Id,
+                                    IntTy Default, DiagnosticsEngine *Diags) {
----------------
tra wrote:
> I'd use anonymous namespace instead of static.
will do


================
Comment at: clang/lib/Basic/OptionUtils.cpp:22
+  if (Arg *A = Args.getLastArg(Id)) {
+    if (StringRef(A->getValue()).getAsInteger(10, Res)) {
+      if (Diags)
----------------
tra wrote:
> Does it have to be base-10?
> Now that the functions are part of Basig API intended for wider use, I'd argue for making it more flexible by default. If specific base is needed, then we could add an argument to specify it.
Added


================
Comment at: clang/lib/Basic/OptionUtils.cpp:33
+
+// Declared in clang/Frontend/Utils.h.
+int getLastArgIntValue(const ArgList &Args, OptSpecifier Id, int Default,
----------------
tra wrote:
> This needs updating.
fixed


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

https://reviews.llvm.org/D71080