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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
