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

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: clang/include/clang/Basic/OptionUtils.h:24
+
+class ArgList;
+
----------------
What are the rules on using LLVM headers here? Can we just include llvm/Option/ArgList.h instead?


================
Comment at: clang/include/clang/Basic/OptionUtils.h:46
+                               DiagnosticsEngine *Diags = nullptr,
+                               unsigned Base = 10);
+
----------------
Same question as before -- does it have to be `10`?
`0` would be a more reasonable default for general use. IMO we care about the value, but not so much about the form. I.e. is there a reason not to allow 0xf, for instance, if that works for the user?



================
Comment at: clang/lib/Basic/CMakeLists.txt:1
 set(LLVM_LINK_COMPONENTS
   Core
----------------
I think now that you're using ArgList, you need to depend on LLVM's LLVMOption library.
As is you're likely to run into build issues if shared libraries are enabled.


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

https://reviews.llvm.org/D71080