[libc-commits] [PATCH] D107792: [libc] add strtoll function and backend

Michael Jones via Phabricator via libc-commits libc-commits at lists.llvm.org
Tue Aug 10 10:54:07 PDT 2021

michaelrj added inline comments.

Comment at: libc/src/__support/stdlib_utils.h:12
+#include "include/errno.h"
+#include "src/__support/ctype_utils.h"
sivachandra wrote:
> We don't include LLVM libc's errno now. Just do `#include <errno.h>` and refer to `errno` directly in code. Lint rules will catch if the header file is not from LLVM libc. For there reference to `errno` in code, you might need a `NOLINT` comment.
> If you do the standard build, as in not the full build, my feeling is this will not build.
I have switched it, but all of the stdlib functions are inside of an `if(LLVM_LIBC_FuLL_BUILD)` in `entrypoints.txt`, so if full build is off then it won't even try to build this.

Comment at: libc/src/__support/stdlib_utils.h:78
+  unsigned long long const CUR_LLONG_MAX =
+      (result_sign == '+' ? LLONG_MAX
sivachandra wrote:
> Nit: What does the `CUR` prefix indicate? It seems to indicate the current value of some variable being updated in a loop. But it is actually a `const`.
In this case it's to distinguish it from LLONG_MAX, since this is the current maximum value we can store (either `LLONG_MAX` or `-LLONG_MIN`). I'm not sure what other prefix to use to indicate that this is a local variable, but a const one.

Comment at: libc/src/__support/stdlib_utils.h:103
+  if (str_end != nullptr)
+    *str_end = (char *)(src);
+  if (result_sign == '+')
This line gives me compiler warnings due to casting from a `const char*` to a `char*` but the specification says that `str_end` is not const. Is there a better way to do this or alternately a way to suppress the compiler warnings?

Comment at: libc/test/src/stdlib/strtoll_test.cpp:53
+  ASSERT_THAT(__llvm_libc::strtoll(too_big_number, &str_end, 10),
+              Fails(ERANGE, __LONG_LONG_MAX__));
+  EXPECT_EQ(str_end - too_big_number, 19l);
sivachandra wrote:
> Why not `LLONG_MAX`?
I was trying to avoid including `<limits.h>`, but on reflection having the variables match across the files is more important than that.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list