[libc-commits] [PATCH] D133163: [libc] add ErrorAnd class to strtointeger

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Thu Sep 1 20:28:02 PDT 2022

sivachandra added inline comments.

Comment at: libc/src/__support/error_and.h:23
+// errno, which may help performance. Additionally, optimization passes may be
+// able to remove the error code if the result is discarded.
+template <typename T> class ErrorAnd {
Going by the use case in `strtointeger`, it doesn't feel like this is the appropriate data structure. The use cases always return a value and some times need an error status also. So, I suggest that strointeger only return a data type like:

<typename T>
struct StrToIntegerResult {
  // Errors during conversion are represented with error values from errno.h.
  int conv_status = 0;
  // Length of the input string parsed.
  size_t parse_length = 0;
  // Conversion result.
  T result = 0;

The `strtointeger` header is modified as:

// As this is a template, we don't need to mark this with `static inline`.
template <typename T>
StrToIntegerResult<T> strtointeger(cpp::string_view src, int base) {

See below for example how this is to be used at call sites.

Comment at: libc/src/__support/str_to_integer.h:155
+  if (result.has_error()) {
+    errno = result.get_error();
+  }
We should not set `errno` in internal functions, especially in `__support`.

Comment at: libc/src/inttypes/strtoimax.cpp:18
                     int base)) {
-  return internal::strtointeger<intmax_t>(str, str_end, base);
+  return internal::strtointeger_errno<intmax_t>(str, str_end, base);
auto result = strtointeger(str, base);
errno = result.conv_status;
*str_end = str + result.parse_length;
return result.conv_val;

It might feel like repeated code, but it was already physically repeated in `strtointeger_errno`. Textual repetition is OK.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list