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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

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.)


Repository:
  rG LLVM Github Monorepo

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

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

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7376d9eb3891: [NFC] Separate getLastArgIntValue to Basic 
(authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71080

Files:
  clang/include/clang/Basic/OptionUtils.h
  clang/include/clang/Frontend/Utils.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/OptionUtils.cpp
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3716,35 +3716,8 @@
   return llvm::APInt(64, code).toString(36, /*Signed=*/false);
 }
 
-template
-static IntTy getLastArgIntValueImpl(const ArgList , 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 , OptSpecifier Id, int Default,
-   DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl(Args, Id, Default, Diags);
-}
-
-uint64_t getLastArgUInt64Value(const ArgList , OptSpecifier Id,
-   uint64_t Default,
-   DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl(Args, Id, Default, Diags);
-}
-
 IntrusiveRefCntPtr
 createVFSFromCompilerInvocation(const CompilerInvocation ,
 DiagnosticsEngine ) {
Index: clang/lib/Basic/OptionUtils.cpp
===
--- /dev/null
+++ clang/lib/Basic/OptionUtils.cpp
@@ -0,0 +1,47 @@
+//===--- 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 
+IntTy getLastArgIntValueImpl(const ArgList , 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 , OptSpecifier Id, int Default,
+   DiagnosticsEngine *Diags, unsigned Base) {
+  return getLastArgIntValueImpl(Args, Id, Default, Diags, Base);
+}
+
+uint64_t getLastArgUInt64Value(const ArgList , OptSpecifier Id,
+   uint64_t Default, DiagnosticsEngine *Diags,
+   unsigned Base) {
+  return getLastArgIntValueImpl(Args, Id, Default, Diags, Base);
+}
+
+} // namespace clang
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   Core
   MC
+  Option
   Support
   )
 
@@ -55,6 +56,7 @@
   ObjCRuntime.cpp
   OpenMPKinds.cpp
   OperatorPrecedence.cpp
+  OptionUtils.cpp
   SanitizerBlacklist.cpp
   SanitizerSpecialCaseList.cpp
   Sanitizers.cpp
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -15,6 +15,7 @@
 
 #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"
@@ -34,12 +35,6 @@
 
 class Triple;
 
-namespace opt {
-
-class ArgList;
-
-} // namespace opt
-
 } // namespace llvm
 
 namespace clang {
@@ -230,29 +225,6 @@
 bool ShouldRecoverOnErrors = false,
 std::vector *CC1Args = nullptr);
 
-/// Return the value of the last argument as an integer, or a default. If Diags
-/// is non-null, emits 

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

ping


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

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 232639.
yaxunl marked 2 inline comments as done.
yaxunl added a comment.

revised by Artem's comments


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

https://reviews.llvm.org/D71080

Files:
  clang/include/clang/Basic/OptionUtils.h
  clang/include/clang/Frontend/Utils.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/OptionUtils.cpp
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3682,35 +3682,8 @@
   return llvm::APInt(64, code).toString(36, /*Signed=*/false);
 }
 
-template
-static IntTy getLastArgIntValueImpl(const ArgList , 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 , OptSpecifier Id, int Default,
-   DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl(Args, Id, Default, Diags);
-}
-
-uint64_t getLastArgUInt64Value(const ArgList , OptSpecifier Id,
-   uint64_t Default,
-   DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl(Args, Id, Default, Diags);
-}
-
 IntrusiveRefCntPtr
 createVFSFromCompilerInvocation(const CompilerInvocation ,
 DiagnosticsEngine ) {
Index: clang/lib/Basic/OptionUtils.cpp
===
--- /dev/null
+++ clang/lib/Basic/OptionUtils.cpp
@@ -0,0 +1,47 @@
+//===--- 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 
+IntTy getLastArgIntValueImpl(const ArgList , 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 , OptSpecifier Id, int Default,
+   DiagnosticsEngine *Diags, unsigned Base) {
+  return getLastArgIntValueImpl(Args, Id, Default, Diags, Base);
+}
+
+uint64_t getLastArgUInt64Value(const ArgList , OptSpecifier Id,
+   uint64_t Default, DiagnosticsEngine *Diags,
+   unsigned Base) {
+  return getLastArgIntValueImpl(Args, Id, Default, Diags, Base);
+}
+
+} // namespace clang
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   Core
   MC
+  Option
   Support
   )
 
@@ -55,6 +56,7 @@
   ObjCRuntime.cpp
   OpenMPKinds.cpp
   OperatorPrecedence.cpp
+  OptionUtils.cpp
   SanitizerBlacklist.cpp
   SanitizerSpecialCaseList.cpp
   Sanitizers.cpp
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -15,6 +15,7 @@
 
 #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"
@@ -34,12 +35,6 @@
 
 class Triple;
 
-namespace opt {
-
-class ArgList;
-
-} // namespace opt
-
 } // namespace llvm
 
 namespace clang {
@@ -230,29 +225,6 @@
 bool ShouldRecoverOnErrors = false,
 std::vector *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 ,

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 5 inline comments as done.
yaxunl added inline comments.



Comment at: clang/include/clang/Basic/OptionUtils.h:24
+
+class ArgList;
+

tra wrote:
> What are the rules on using LLVM headers here? Can we just include 
> llvm/Option/ArgList.h instead?
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.



Comment at: clang/include/clang/Basic/OptionUtils.h:46
+   DiagnosticsEngine *Diags = nullptr,
+   unsigned Base = 10);
+

tra wrote:
> 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?
> 
will do. StringRef will do radix autosensing for that.



Comment at: clang/lib/Basic/CMakeLists.txt:1
 set(LLVM_LINK_COMPONENTS
   Core

tra wrote:
> 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.
will do


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

[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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 
+static IntTy getLastArgIntValueImpl(const ArgList , 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 , 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 232592.
yaxunl marked 4 inline comments as done.
yaxunl added a comment.

revised by Artem's comments.


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

https://reviews.llvm.org/D71080

Files:
  clang/include/clang/Basic/OptionUtils.h
  clang/include/clang/Frontend/Utils.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/OptionUtils.cpp
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3682,35 +3682,8 @@
   return llvm::APInt(64, code).toString(36, /*Signed=*/false);
 }
 
-template
-static IntTy getLastArgIntValueImpl(const ArgList , 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 , OptSpecifier Id, int Default,
-   DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl(Args, Id, Default, Diags);
-}
-
-uint64_t getLastArgUInt64Value(const ArgList , OptSpecifier Id,
-   uint64_t Default,
-   DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl(Args, Id, Default, Diags);
-}
-
 IntrusiveRefCntPtr
 createVFSFromCompilerInvocation(const CompilerInvocation ,
 DiagnosticsEngine ) {
Index: clang/lib/Basic/OptionUtils.cpp
===
--- /dev/null
+++ clang/lib/Basic/OptionUtils.cpp
@@ -0,0 +1,47 @@
+//===--- 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 
+IntTy getLastArgIntValueImpl(const ArgList , 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 , OptSpecifier Id, int Default,
+   DiagnosticsEngine *Diags, unsigned Base) {
+  return getLastArgIntValueImpl(Args, Id, Default, Diags, Base);
+}
+
+uint64_t getLastArgUInt64Value(const ArgList , OptSpecifier Id,
+   uint64_t Default, DiagnosticsEngine *Diags,
+   unsigned Base) {
+  return getLastArgIntValueImpl(Args, Id, Default, Diags, Base);
+}
+
+} // namespace clang
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -55,6 +55,7 @@
   ObjCRuntime.cpp
   OpenMPKinds.cpp
   OperatorPrecedence.cpp
+  OptionUtils.cpp
   SanitizerBlacklist.cpp
   SanitizerSpecialCaseList.cpp
   Sanitizers.cpp
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -15,6 +15,7 @@
 
 #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"
@@ -34,12 +35,6 @@
 
 class Triple;
 
-namespace opt {
-
-class ArgList;
-
-} // namespace opt
-
 } // namespace llvm
 
 namespace clang {
@@ -230,29 +225,6 @@
 bool ShouldRecoverOnErrors = false,
 std::vector *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 ,
-   llvm::opt::OptSpecifier Id, int Default,
- 

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

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Basic/OptionUtils.cpp:18
+template 
+static IntTy getLastArgIntValueImpl(const ArgList , OptSpecifier Id,
+IntTy Default, DiagnosticsEngine *Diags) {

I'd use anonymous namespace instead of static.



Comment at: clang/lib/Basic/OptionUtils.cpp:22
+  if (Arg *A = Args.getLastArg(Id)) {
+if (StringRef(A->getValue()).getAsInteger(10, Res)) {
+  if (Diags)

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.



Comment at: clang/lib/Basic/OptionUtils.cpp:33
+
+// Declared in clang/Frontend/Utils.h.
+int getLastArgIntValue(const ArgList , OptSpecifier Id, int Default,

This needs updating.


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

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall, rsmith.
Herald added a subscriber: mgorny.

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.


https://reviews.llvm.org/D71080

Files:
  clang/include/clang/Basic/OptionUtils.h
  clang/include/clang/Frontend/Utils.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/OptionUtils.cpp
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3682,35 +3682,8 @@
   return llvm::APInt(64, code).toString(36, /*Signed=*/false);
 }
 
-template
-static IntTy getLastArgIntValueImpl(const ArgList , 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 , OptSpecifier Id, int Default,
-   DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl(Args, Id, Default, Diags);
-}
-
-uint64_t getLastArgUInt64Value(const ArgList , OptSpecifier Id,
-   uint64_t Default,
-   DiagnosticsEngine *Diags) {
-  return getLastArgIntValueImpl(Args, Id, Default, Diags);
-}
-
 IntrusiveRefCntPtr
 createVFSFromCompilerInvocation(const CompilerInvocation ,
 DiagnosticsEngine ) {
Index: clang/lib/Basic/OptionUtils.cpp
===
--- /dev/null
+++ clang/lib/Basic/OptionUtils.cpp
@@ -0,0 +1,44 @@
+//===--- 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;
+
+template 
+static IntTy getLastArgIntValueImpl(const ArgList , 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 , OptSpecifier Id, int Default,
+   DiagnosticsEngine *Diags) {
+  return getLastArgIntValueImpl(Args, Id, Default, Diags);
+}
+
+uint64_t getLastArgUInt64Value(const ArgList , OptSpecifier Id,
+   uint64_t Default, DiagnosticsEngine *Diags) {
+  return getLastArgIntValueImpl(Args, Id, Default, Diags);
+}
+
+} // namespace clang
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -55,6 +55,7 @@
   ObjCRuntime.cpp
   OpenMPKinds.cpp
   OperatorPrecedence.cpp
+  OptionUtils.cpp
   SanitizerBlacklist.cpp
   SanitizerSpecialCaseList.cpp
   Sanitizers.cpp
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -15,6 +15,7 @@
 
 #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"
@@ -34,12 +35,6 @@
 
 class Triple;
 
-namespace opt {
-
-class ArgList;
-
-} // namespace opt
-
 } // namespace llvm
 
 namespace clang {
@@ -230,29 +225,6 @@
 bool ShouldRecoverOnErrors = false,
 std::vector *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 ,
-