[NFC] Separate getLastArgIntValue to Basic

[yaxunl]

Summary

getLastArgIntValue is a useful utility function to get command line argument as an integer.
Currently it is in Frontend so that it can only be used by clang -cc1. Move it to basic so
that it can also be used by clang driver.

[tra]

clang/lib/Basic/OptionUtils.cpp

19

I'd use anonymous namespace instead of static.

23

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.

34

This needs updating.

[yaxunl]

revised by Artem's comments.

clang/lib/Basic/OptionUtils.cpp

19

will do

23

Added

34

fixed

[tra]

clang/include/clang/Basic/OptionUtils.h

25

What are the rules on using LLVM headers here? Can we just include llvm/Option/ArgList.h instead?

47

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?

clang/lib/Basic/CMakeLists.txt

1

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.

[yaxunl]

clang/include/clang/Basic/OptionUtils.h

25

In the API llvm::opt::ArgList is used as reference, therefore it only needs a forward declaration. llvm::opt::OptSpecifier is passed by value, therefore it needs header. OptSpecifier is a small class, therefore it is not efficient to pass by reference.

47

will do. StringRef will do radix autosensing for that.

clang/lib/Basic/CMakeLists.txt

1

will do

[yaxunl]

revised by Artem's comments

[yaxunl]

ping

[thakis]

Why move this to Basic instead of to Driver if you want to use it in Driver? (Frontend depends on Driver so it could then still use it.)

[yaxunl]

In D71080#1794066, @thakis wrote:

Why move this to Basic instead of to Driver if you want to use it in Driver? (Frontend depends on Driver so it could then still use it.)

You are right. Driver is a better place. I will create a review for it.

[thakis]

(The follow-up is at D71802. Thanks!)

clang/include/clang/Basic/OptionUtils.h

+//===- OptionUtils.h - Utilities for command line arguments -----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//  This header contains utilities for command line arguments.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_BASIC_OPTIONUTILS_H
+#define LLVM_CLANG_BASIC_OPTIONUTILS_H
+
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/Option/OptSpecifier.h"
+
+namespace llvm {
+
+namespace opt {
+
+class ArgList;
+
+} // namespace opt
+
+} // namespace llvm
+
+namespace clang {
+/// Return the value of the last argument as an integer, or a default. If Diags
+/// is non-null, emits an error if the argument is given, but non-integral.
+int getLastArgIntValue(const llvm::opt::ArgList &Args,
+                       llvm::opt::OptSpecifier Id, int Default,
+                       DiagnosticsEngine *Diags = nullptr, unsigned Base = 0);
+
+inline int getLastArgIntValue(const llvm::opt::ArgList &Args,
+                              llvm::opt::OptSpecifier Id, int Default,
+                              DiagnosticsEngine &Diags, unsigned Base = 0) {
+  return getLastArgIntValue(Args, Id, Default, &Diags, Base);
+}
+
+uint64_t getLastArgUInt64Value(const llvm::opt::ArgList &Args,
+                               llvm::opt::OptSpecifier Id, uint64_t Default,
+                               DiagnosticsEngine *Diags = nullptr,
+                               unsigned Base = 0);
+
+inline uint64_t getLastArgUInt64Value(const llvm::opt::ArgList &Args,
+                                      llvm::opt::OptSpecifier Id,
+                                      uint64_t Default,
+                                      DiagnosticsEngine &Diags,
+                                      unsigned Base = 0) {
+  return getLastArgUInt64Value(Args, Id, Default, &Diags, Base);
+}
+
+} // namespace clang
+
+#endif // LLVM_CLANG_BASIC_OPTIONUTILS_H

clang/include/clang/Frontend/Utils.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_FRONTEND_UTILS_H
 #define LLVM_CLANG_FRONTEND_UTILS_H
 
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/OptionUtils.h"
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Option/OptSpecifier.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include <cstdint>
 #include <memory>
 #include <string>
 #include <system_error>
 #include <utility>
 #include <vector>
 
 namespace llvm {
 
 class Triple;
 
-namespace opt {
-
-class ArgList;
-
-} // namespace opt
-
 } // namespace llvm
 
 namespace clang {
 
 class ASTReader;
 class CompilerInstance;
 class CompilerInvocation;
 class DiagnosticsEngine;

 std::unique_ptr<CompilerInvocation> createInvocationFromCommandLine(
     ArrayRef<const char *> Args,
     IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
         IntrusiveRefCntPtr<DiagnosticsEngine>(),
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr,
     bool ShouldRecoverOnErrors = false,
     std::vector<std::string> *CC1Args = nullptr);
 
-/// Return the value of the last argument as an integer, or a default. If Diags
-/// is non-null, emits an error if the argument is given, but non-integral.
-int getLastArgIntValue(const llvm::opt::ArgList &Args,
-                       llvm::opt::OptSpecifier Id, int Default,
-                       DiagnosticsEngine *Diags = nullptr);
-
-inline int getLastArgIntValue(const llvm::opt::ArgList &Args,
-                              llvm::opt::OptSpecifier Id, int Default,
-                              DiagnosticsEngine &Diags) {
-  return getLastArgIntValue(Args, Id, Default, &Diags);
-}
-
-uint64_t getLastArgUInt64Value(const llvm::opt::ArgList &Args,
-                               llvm::opt::OptSpecifier Id, uint64_t Default,
-                               DiagnosticsEngine *Diags = nullptr);
-
-inline uint64_t getLastArgUInt64Value(const llvm::opt::ArgList &Args,
-                                      llvm::opt::OptSpecifier Id,
-                                      uint64_t Default,
-                                      DiagnosticsEngine &Diags) {
-  return getLastArgUInt64Value(Args, Id, Default, &Diags);
-}
-
 // Frontend timing utils
 
 /// If the user specifies the -ftime-report argument on an Clang command line
 /// then the value of this boolean will be true, otherwise false.
 extern bool FrontendTimesIsEnabled;
 
 } // namespace clang
 
 #endif // LLVM_CLANG_FRONTEND_UTILS_H

clang/lib/Basic/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Core
   MC
+  Option
   Support
   )
 
 find_first_existing_vc_file("${LLVM_MAIN_SRC_DIR}" llvm_vc)
 find_first_existing_vc_file("${CLANG_SOURCE_DIR}" clang_vc)
 
 # The VC revision include that we want to generate.
 set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSVersion.inc")
add_clang_library(clangBasic
   FixedPoint.cpp
   IdentifierTable.cpp
   LangOptions.cpp
   LangStandards.cpp
   Module.cpp
   ObjCRuntime.cpp
   OpenMPKinds.cpp
   OperatorPrecedence.cpp
+  OptionUtils.cpp
   SanitizerBlacklist.cpp
   SanitizerSpecialCaseList.cpp
   Sanitizers.cpp
   SourceLocation.cpp
   SourceManager.cpp
   Stack.cpp
   TargetInfo.cpp
   Targets.cpp

clang/lib/Basic/OptionUtils.cpp

+//===--- OptionUtils.cpp - Utilities for command line arguments -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/OptionUtils.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticDriver.h"
+#include "llvm/Option/ArgList.h"
+
+using namespace clang;
+using namespace llvm::opt;
+
+namespace {
+template <typename IntTy>
+IntTy getLastArgIntValueImpl(const ArgList &Args, OptSpecifier Id,
+                             IntTy Default, DiagnosticsEngine *Diags,
+                             unsigned Base) {
+  IntTy Res = Default;
+  if (Arg *A = Args.getLastArg(Id)) {
+    if (StringRef(A->getValue()).getAsInteger(Base, Res)) {
+      if (Diags)
+        Diags->Report(diag::err_drv_invalid_int_value)
+            << A->getAsString(Args) << A->getValue();
+    }
+  }
+  return Res;
+}
+} // namespace
+
+namespace clang {
+
+int getLastArgIntValue(const ArgList &Args, OptSpecifier Id, int Default,
+                       DiagnosticsEngine *Diags, unsigned Base) {
+  return getLastArgIntValueImpl<int>(Args, Id, Default, Diags, Base);
+}
+
+uint64_t getLastArgUInt64Value(const ArgList &Args, OptSpecifier Id,
+                               uint64_t Default, DiagnosticsEngine *Diags,
+                               unsigned Base) {
+  return getLastArgIntValueImpl<uint64_t>(Args, Id, Default, Diags, Base);
+}
+
+} // namespace clang

clang/lib/Frontend/CompilerInvocation.cpp

#include "clang/Basic/LangOptions.def"
   SanitizerSet SanHash = LangOpts->Sanitize;
   SanHash.clear(getPPTransparentSanitizers());
   if (!SanHash.empty())
     code = hash_combine(code, SanHash.Mask);
 
   return llvm::APInt(64, code).toString(36, /*Signed=*/false);
 }
 
-template<typename IntTy>
-static IntTy getLastArgIntValueImpl(const ArgList &Args, OptSpecifier Id,
-                                    IntTy Default,
-                                    DiagnosticsEngine *Diags) {
-  IntTy Res = Default;
-  if (Arg *A = Args.getLastArg(Id)) {
-    if (StringRef(A->getValue()).getAsInteger(10, Res)) {
-      if (Diags)
-        Diags->Report(diag::err_drv_invalid_int_value) << A->getAsString(Args)
-                                                       << A->getValue();
-    }
-  }
-  return Res;
-}
-
 namespace clang {
 
-// Declared in clang/Frontend/Utils.h.
-int getLastArgIntValue(const ArgList &Args, OptSpecifier Id, int Default,
-                       DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl<int>(Args, Id, Default, Diags);
-}
-
-uint64_t getLastArgUInt64Value(const ArgList &Args, OptSpecifier Id,
-                               uint64_t Default,
-                               DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl<uint64_t>(Args, Id, Default, Diags);
-}
-
 IntrusiveRefCntPtr<llvm::vfs::FileSystem>
 createVFSFromCompilerInvocation(const CompilerInvocation &CI,
                                 DiagnosticsEngine &Diags) {
   return createVFSFromCompilerInvocation(CI, Diags,
                                          llvm::vfs::getRealFileSystem());
 }
 
 IntrusiveRefCntPtr<llvm::vfs::FileSystem> createVFSFromCompilerInvocation(