[libc] Fix casts for arm32 after Wconversion

[michaelrj-google]

Followup to #127523

There were some test failures on arm32 after enabling Wconversion. There
were some tests that were failing due to missing casts. Also I changed
BigInt's safe_get_at back to being signed since it needed the ability
to be negative.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

Followup to #127523

There were some test failures on arm32 after enabling Wconversion. There
were some tests that were failing due to missing casts. Also I changed
BigInt's safe_get_at back to being signed since it needed the ability
to be negative.

---

Full diff: https://github.com/llvm/llvm-project/pull/129771.diff

2 Files Affected:

• (modified) libc/src/__support/big_int.h (+5-6)
• (modified) libc/src/string/memory_utils/generic/byte_per_byte.h (+2-1)

diff --git a/libc/src/__support/big_int.h b/libc/src/__support/big_int.h
index 85a17f3110d6a..f44624a7eafce 100644
--- a/libc/src/__support/big_int.h
+++ b/libc/src/__support/big_int.h
@@ -272,15 +272,15 @@ LIBC_INLINE constexpr cpp::array<word, N> shift(cpp::array<word, N> array,
   if (LIBC_UNLIKELY(offset == 0))
     return array;
   const bool is_neg = is_signed && is_negative(array);
-  constexpr auto at = [](size_t index) -> size_t {
+  constexpr auto at = [](size_t index) -> int {
     // reverse iteration when direction == LEFT.
     if constexpr (direction == LEFT)
-      return N - index - 1;
-    return index;
+      return int(N) - int(index) - 1;
+    return int(index);
   };
   const auto safe_get_at = [&](size_t index) -> word {
     // return appropriate value when accessing out of bound elements.
-    const size_t i = at(index);
+    const int i = at(index);
     if (i < 0)
       return 0;
     if (i >= int(N))
@@ -465,8 +465,7 @@ struct BigInt {
   }
 
   // Initialize the first word to |v| and the rest to 0.
-  template <typename T, typename = cpp::enable_if_t<cpp::is_integral_v<T> &&
-                                                    !cpp::is_same_v<T, bool>>>
+  template <typename T, typename = cpp::enable_if_t<cpp::is_integral_v<T>>>
   LIBC_INLINE constexpr BigInt(T v) {
     constexpr size_t T_SIZE = sizeof(T) * CHAR_BIT;
     const bool is_neg = v < 0;
diff --git a/libc/src/string/memory_utils/generic/byte_per_byte.h b/libc/src/string/memory_utils/generic/byte_per_byte.h
index 2aecf0126cc43..862aeecab7f55 100644
--- a/libc/src/string/memory_utils/generic/byte_per_byte.h
+++ b/libc/src/string/memory_utils/generic/byte_per_byte.h
@@ -38,7 +38,8 @@ inline_memmove_byte_per_byte(Ptr dst, CPtr src, size_t count) {
       dst[offset] = src[offset];
   } else {
     LIBC_LOOP_NOUNROLL
-    for (ptrdiff_t offset = count - 1; offset >= 0; --offset)
+    for (ptrdiff_t offset = static_cast<ptrdiff_t>(count - 1); offset >= 0;
+         --offset)
       dst[offset] = src[offset];
   }
 }

[Caslyn]

Thanks for the fix! We saw similar breakages on our Fuchsia-libc integration roller. LGTM

[michaelrj-google]

Thanks for the review, I'll merge after presubmits finish.

[vinay-deshmukh]

@michaelrj-google Thanks for the quick fix!

Do you know why the arm32 issues weren't caught in the checks that run on the PR? All of those seemed to be passing.

back to being signed since it needed the ability to be negative.

TIL, is it to have indexing like Python does where -1 can refer to the "last" element?

Follow up: how does a negative value work here, given that cpp::array::operator[] takes only a size_t as argument:

llvm-project/libc/src/__support/CPP/array.h

Lines 40 to 44 in 3b38992

	LIBC_INLINE constexpr T &operator[](size_t Index) { return Data[Index]; }
	LIBC_INLINE constexpr const T &operator[](size_t Index) const {
	return Data[Index];
	}

Aside: I'll be running into the same/similar warning in: #129811

[michaelrj-google]

It's not that array handles negatives, it's that the at lambda is only used in safe_get_at, and the safe_get_at function does the bounds checking:

  const auto safe_get_at = [&](size_t index) -> word {
    // return appropriate value when accessing out of bound elements.
    const int i = at(index);
    if (i < 0)
      return 0;
    if (i >= int(N))
      return is_neg ? cpp::numeric_limits<word>::max() : 0;
    return array[i];
  };

If i is negative or greater than the size of the size of the array, it returns before it attempts to index into array.