[libc-commits] [PATCH] D107999: [libc] Add strtol, strtoul, and strtoull

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Mon Aug 16 22:04:54 PDT 2021

sivachandra accepted this revision.
sivachandra added inline comments.
This revision is now accepted and ready to land.

Comment at: libc/src/__support/str_conv_utils.h:120
+  return (result_sign == '+') ? static_cast<T>(result)
+                              : -static_cast<T>(result);
michaelrj wrote:
> sivachandra wrote:
> > When the sign is negative and result is `ABS_MAX` and the return type is a signed integer type, then I think we have to handle that specially. Consider this: T is `long`, `result_sign` is `-` and `result` is `ABS_MAX` which is effectively `LONG_MAX + 1`. Since `LONG_MAX + 1` cannot be represent as a `long` type, the `static_cast` here becomes an implementation defined operation.
> > 
> > I think previously also it was implementation defined.
> > 
> You're right, I've added an extra case and a comment explaining it.
Can we may be handle the overflow case like this:

if (result == ABS_MAX) {
  if (result_sign == '+' || is_unsigned) 
     return __llvm_libc::cpp::NumericLimits<T>::max();
  else // T is signed and there is a negative overflow
     return __llvm_libc::cpp::NumericLimits<T>::min();

For better readability, before line 78 you can define the following and use it above and also on line 79.

constexpr bool is_unsigned = (__llvm_libc::cpp::NumericLimits<T>::min() == 0);

Along the same lines, you can probably also define:

const bool is_positive = (result_sign == '+');

Comment at: libc/src/__support/str_conv_utils.h:78
+  unsigned long long const NEGATIVE_MAX =
+      (__llvm_libc::cpp::NumericLimits<T>::min() != 0)
This can be `constexpr`.

Comment at: libc/src/__support/str_conv_utils.h:94
+    ++src;
+    if (result == ABS_MAX)
Add a comment here explaining the early return; point out that we read out the full number even if it is out of range.

Comment at: libc/src/stdlib/CMakeLists.txt:93
\ No newline at end of file

Fix here and in many other files.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list