Skip to content

[libc] Enable -Wconversion for tests. #127523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 69 commits into from
Mar 4, 2025

Conversation

vinay-deshmukh
Copy link
Contributor

Relates to: #119281

Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

Copy link

github-actions bot commented Feb 23, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@lntue lntue added the libc label Feb 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2025

@llvm/pr-subscribers-libc

Author: Vinay Deshmukh (vinay-deshmukh)

Changes

Relates to: #119281


Patch is 72.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/127523.diff

47 Files Affected:

  • (modified) libc/cmake/modules/LLVMLibCTestRules.cmake (+1-2)
  • (modified) libc/src/__support/CPP/bit.h (+4-4)
  • (modified) libc/src/__support/CPP/span.h (+4-2)
  • (modified) libc/src/__support/CPP/string.h (+2-1)
  • (modified) libc/src/__support/CPP/string_view.h (+4-2)
  • (modified) libc/src/__support/FPUtil/FPBits.h (+3-3)
  • (modified) libc/src/__support/FPUtil/NormalFloat.h (+2-2)
  • (modified) libc/src/__support/FPUtil/aarch64/FEnvImpl.h (+7-5)
  • (modified) libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h (+23-16)
  • (modified) libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h (+1-1)
  • (modified) libc/src/__support/OSUtil/darwin/io.h (+2-1)
  • (modified) libc/src/__support/big_int.h (+17-14)
  • (modified) libc/src/__support/high_precision_decimal.h (+7-7)
  • (modified) libc/src/__support/integer_literals.h (+1-1)
  • (modified) libc/src/__support/integer_to_string.h (+3-2)
  • (modified) libc/src/__support/memory_size.h (+2-1)
  • (modified) libc/src/__support/str_to_float.h (+12-12)
  • (modified) libc/src/__support/str_to_integer.h (+6-6)
  • (modified) libc/src/stdio/printf_core/parser.h (+10-8)
  • (modified) libc/src/stdio/printf_core/writer.h (+2-1)
  • (modified) libc/src/stdio/scanf_core/parser.h (+7-6)
  • (modified) libc/src/stdlib/quick_sort.h (+3-1)
  • (modified) libc/src/string/memory_utils/utils.h (+2-2)
  • (modified) libc/src/string/string_utils.h (+8-6)
  • (modified) libc/test/UnitTest/ExecuteFunction.h (+4-4)
  • (modified) libc/test/UnitTest/ExecuteFunctionUnix.cpp (+1-1)
  • (modified) libc/test/UnitTest/LibcTest.cpp (+2-2)
  • (modified) libc/test/UnitTest/MemoryMatcher.h (+1-1)
  • (modified) libc/test/src/__support/CPP/bit_test.cpp (+10-7)
  • (modified) libc/test/src/__support/arg_list_test.cpp (+5-4)
  • (modified) libc/test/src/__support/big_int_test.cpp (+4-4)
  • (modified) libc/test/src/__support/blockstore_test.cpp (+1-1)
  • (modified) libc/test/src/__support/fixedvector_test.cpp (+2-2)
  • (modified) libc/test/src/__support/hash_test.cpp (+1-1)
  • (modified) libc/test/src/__support/integer_to_string_test.cpp (+6-5)
  • (modified) libc/test/src/__support/math_extras_test.cpp (+17-11)
  • (modified) libc/test/src/__support/str_to_double_test.cpp (+1-1)
  • (modified) libc/test/src/__support/str_to_float_test.cpp (+1-1)
  • (modified) libc/test/src/__support/str_to_fp_test.h (+3-3)
  • (modified) libc/test/src/math/FModTest.h (+4-4)
  • (modified) libc/test/src/stdio/printf_core/parser_test.cpp (+40-20)
  • (modified) libc/test/src/string/memmove_test.cpp (+2-1)
  • (modified) libc/test/src/string/memory_utils/memory_check_utils.h (+5-3)
  • (modified) libc/test/src/string/memory_utils/op_tests.cpp (+2-2)
  • (modified) libc/test/src/string/memory_utils/utils_test.cpp (+1-1)
  • (modified) libc/test/src/string/memset_test.cpp (+1-1)
  • (modified) libc/test/src/strings/bcopy_test.cpp (+2-1)
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 03b4b251649e7..03b76508fdf6c 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -36,8 +36,7 @@ function(_get_common_test_compile_options output_var c_test flags)
     if(NOT LIBC_WNO_ERROR)
       # list(APPEND compile_options "-Werror")
     endif()
-    # list(APPEND compile_options "-Wconversion")
-    # list(APPEND compile_options "-Wno-sign-conversion")
+    list(APPEND compile_options "-Wconversion")
     list(APPEND compile_options "-Wimplicit-fallthrough")
     list(APPEND compile_options "-Wwrite-strings")
     # Silence this warning because _Complex is a part of C99.
diff --git a/libc/src/__support/CPP/bit.h b/libc/src/__support/CPP/bit.h
index 82b9eb5128262..7d138201783bf 100644
--- a/libc/src/__support/CPP/bit.h
+++ b/libc/src/__support/CPP/bit.h
@@ -101,7 +101,7 @@ countr_zero(T value) {
     shift >>= 1;
     mask >>= shift;
   }
-  return zero_bits;
+  return static_cast<int>(zero_bits);
 }
 #if __has_builtin(__builtin_ctzs)
 ADD_SPECIALIZATION(countr_zero, unsigned short, __builtin_ctzs)
@@ -140,7 +140,7 @@ countl_zero(T value) {
     else
       zero_bits |= shift;
   }
-  return zero_bits;
+  return static_cast<int>(zero_bits);
 }
 #if __has_builtin(__builtin_clzs)
 ADD_SPECIALIZATION(countl_zero, unsigned short, __builtin_clzs)
@@ -226,7 +226,7 @@ rotr(T value, int rotate);
 template <typename T>
 [[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
 rotl(T value, int rotate) {
-  constexpr unsigned N = cpp::numeric_limits<T>::digits;
+  constexpr int N = cpp::numeric_limits<T>::digits;
   rotate = rotate % N;
   if (!rotate)
     return value;
@@ -238,7 +238,7 @@ rotl(T value, int rotate) {
 template <typename T>
 [[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
 rotr(T value, int rotate) {
-  constexpr unsigned N = cpp::numeric_limits<T>::digits;
+  constexpr int N = cpp::numeric_limits<T>::digits;
   rotate = rotate % N;
   if (!rotate)
     return value;
diff --git a/libc/src/__support/CPP/span.h b/libc/src/__support/CPP/span.h
index a41c9b744e370..9234a26d201cd 100644
--- a/libc/src/__support/CPP/span.h
+++ b/libc/src/__support/CPP/span.h
@@ -11,6 +11,7 @@
 #include <stddef.h> // For size_t
 
 #include "array.h" // For array
+#include "limits.h"
 #include "src/__support/macros/config.h"
 #include "type_traits.h" // For remove_cv_t, enable_if_t, is_same_v, is_const_v
 
@@ -48,7 +49,8 @@ template <typename T> class span {
   using const_reference = const T &;
   using iterator = T *;
 
-  LIBC_INLINE_VAR static constexpr size_type dynamic_extent = -1;
+  LIBC_INLINE_VAR static constexpr size_type dynamic_extent =
+      cpp::numeric_limits<size_type>::max();
 
   LIBC_INLINE constexpr span() : span_data(nullptr), span_size(0) {}
 
@@ -58,7 +60,7 @@ template <typename T> class span {
       : span_data(first), span_size(count) {}
 
   LIBC_INLINE constexpr span(pointer first, pointer end)
-      : span_data(first), span_size(end - first) {}
+      : span_data(first), span_size(static_cast<size_t>(end - first)) {}
 
   template <typename U, size_t N,
             cpp::enable_if_t<is_compatible_v<U>, bool> = true>
diff --git a/libc/src/__support/CPP/string.h b/libc/src/__support/CPP/string.h
index dbc0ae04e5e6f..1ac04c7f1f9dc 100644
--- a/libc/src/__support/CPP/string.h
+++ b/libc/src/__support/CPP/string.h
@@ -67,7 +67,8 @@ class string {
       : string(cstr, ::LIBC_NAMESPACE::internal::string_length(cstr)) {}
   LIBC_INLINE string(size_t size_, char value) {
     resize(size_);
-    inline_memset((void *)buffer_, value, size_);
+    static_assert(sizeof(char) == sizeof(uint8_t));
+    inline_memset((void *)buffer_, static_cast<uint8_t>(value), size_);
   }
 
   LIBC_INLINE string &operator=(const string &other) {
diff --git a/libc/src/__support/CPP/string_view.h b/libc/src/__support/CPP/string_view.h
index 745c62c35f0a0..aa15814b2e149 100644
--- a/libc/src/__support/CPP/string_view.h
+++ b/libc/src/__support/CPP/string_view.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_LIBC_SRC___SUPPORT_CPP_STRING_VIEW_H
 #define LLVM_LIBC_SRC___SUPPORT_CPP_STRING_VIEW_H
 
+#include "limits.h"
 #include "src/__support/common.h"
 #include "src/__support/macros/config.h"
 
@@ -40,7 +41,7 @@ class string_view {
   LIBC_INLINE static constexpr size_t length(const char *Str) {
     for (const char *End = Str;; ++End)
       if (*End == '\0')
-        return End - Str;
+        return static_cast<size_t>(End - Str);
   }
 
   LIBC_INLINE bool equals(string_view Other) const {
@@ -61,7 +62,8 @@ class string_view {
 
   // special value equal to the maximum value representable by the type
   // size_type.
-  LIBC_INLINE_VAR static constexpr size_t npos = -1;
+  LIBC_INLINE_VAR static constexpr size_t npos =
+      cpp::numeric_limits<size_t>::max();
 
   LIBC_INLINE constexpr string_view() : Data(nullptr), Len(0) {}
 
diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 90b6e406e0f31..bee8d0a8dc47d 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -247,11 +247,11 @@ template <FPType fp_type> struct FPStorage : public FPLayout<fp_type> {
     using UP::UP;
 
     LIBC_INLINE constexpr BiasedExponent(Exponent exp)
-        : UP(static_cast<int32_t>(exp) + EXP_BIAS) {}
+        : UP(static_cast<uint32_t>(static_cast<int32_t>(exp) + EXP_BIAS)) {}
 
     // Cast operator to get convert from BiasedExponent to Exponent.
     LIBC_INLINE constexpr operator Exponent() const {
-      return Exponent(UP::value - EXP_BIAS);
+      return Exponent(static_cast<int32_t>(UP::value - EXP_BIAS));
     }
 
     LIBC_INLINE constexpr BiasedExponent &operator++() {
@@ -686,7 +686,7 @@ struct FPRepImpl : public FPRepSem<fp_type, RetT> {
   }
 
   LIBC_INLINE constexpr void set_biased_exponent(StorageType biased) {
-    UP::set_biased_exponent(BiasedExponent((int32_t)biased));
+    UP::set_biased_exponent(BiasedExponent(static_cast<uint32_t>(biased)));
   }
 
   LIBC_INLINE constexpr int get_exponent() const {
diff --git a/libc/src/__support/FPUtil/NormalFloat.h b/libc/src/__support/FPUtil/NormalFloat.h
index b4cbb5042a68b..a2f285fc6fb95 100644
--- a/libc/src/__support/FPUtil/NormalFloat.h
+++ b/libc/src/__support/FPUtil/NormalFloat.h
@@ -105,7 +105,7 @@ template <typename T> struct NormalFloat {
 
     constexpr int SUBNORMAL_EXPONENT = -FPBits<T>::EXP_BIAS + 1;
     if (exponent < SUBNORMAL_EXPONENT) {
-      unsigned shift = SUBNORMAL_EXPONENT - exponent;
+      unsigned shift = static_cast<unsigned>(SUBNORMAL_EXPONENT - exponent);
       // Since exponent > subnormalExponent, shift is strictly greater than
       // zero.
       if (shift <= FPBits<T>::FRACTION_LEN + 1) {
@@ -160,7 +160,7 @@ template <typename T> struct NormalFloat {
     if (bits.is_subnormal()) {
       unsigned shift = evaluate_normalization_shift(bits.get_mantissa());
       mantissa = static_cast<StorageType>(bits.get_mantissa() << shift);
-      exponent = 1 - FPBits<T>::EXP_BIAS - shift;
+      exponent = 1 - FPBits<T>::EXP_BIAS - static_cast<int32_t>(shift);
     } else {
       exponent = bits.get_biased_exponent() - FPBits<T>::EXP_BIAS;
       mantissa = ONE | bits.get_mantissa();
diff --git a/libc/src/__support/FPUtil/aarch64/FEnvImpl.h b/libc/src/__support/FPUtil/aarch64/FEnvImpl.h
index 18b0631324f8f..914155a01631d 100644
--- a/libc/src/__support/FPUtil/aarch64/FEnvImpl.h
+++ b/libc/src/__support/FPUtil/aarch64/FEnvImpl.h
@@ -110,7 +110,7 @@ LIBC_INLINE int enable_except(int excepts) {
       (controlWord >> FEnv::ExceptionControlFlagsBitPosition) & 0x1F;
   controlWord |= (newExcepts << FEnv::ExceptionControlFlagsBitPosition);
   FEnv::writeControlWord(controlWord);
-  return FEnv::exceptionStatusToMacro(oldExcepts);
+  return FEnv::exceptionStatusToMacro(static_cast<uint32_t>(oldExcepts));
 }
 
 LIBC_INLINE int disable_except(int excepts) {
@@ -120,12 +120,12 @@ LIBC_INLINE int disable_except(int excepts) {
       (controlWord >> FEnv::ExceptionControlFlagsBitPosition) & 0x1F;
   controlWord &= ~(disabledExcepts << FEnv::ExceptionControlFlagsBitPosition);
   FEnv::writeControlWord(controlWord);
-  return FEnv::exceptionStatusToMacro(oldExcepts);
+  return FEnv::exceptionStatusToMacro(static_cast<uint32_t>(oldExcepts));
 }
 
 LIBC_INLINE int get_except() {
   uint32_t controlWord = FEnv::getControlWord();
-  int enabledExcepts =
+  uint32_t enabledExcepts =
       (controlWord >> FEnv::ExceptionControlFlagsBitPosition) & 0x1F;
   return FEnv::exceptionStatusToMacro(enabledExcepts);
 }
@@ -250,8 +250,10 @@ LIBC_INLINE int set_round(int mode) {
   }
 
   uint32_t controlWord = FEnv::getControlWord();
-  controlWord &= ~(0x3 << FEnv::RoundingControlBitPosition);
-  controlWord |= (bitValue << FEnv::RoundingControlBitPosition);
+  controlWord &=
+      static_cast<uint32_t>(~(0x3 << FEnv::RoundingControlBitPosition));
+  controlWord |=
+      static_cast<uint32_t>(bitValue << FEnv::RoundingControlBitPosition);
   FEnv::writeControlWord(controlWord);
 
   return 0;
diff --git a/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h b/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h
index 969e70796d1f1..dcce76b6116be 100644
--- a/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h
+++ b/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h
@@ -63,7 +63,7 @@ struct FEnv {
   // __fpcr_flush_to_zero bit in the FPCR register.  This control bit is
   // located in a different place from FE_FLUSHTOZERO status bit relative to
   // the other exceptions.
-  LIBC_INLINE static uint32_t exception_value_from_status(int status) {
+  LIBC_INLINE static uint32_t exception_value_from_status(uint32_t status) {
     return ((status & FE_INVALID) ? EX_INVALID : 0) |
            ((status & FE_DIVBYZERO) ? EX_DIVBYZERO : 0) |
            ((status & FE_OVERFLOW) ? EX_OVERFLOW : 0) |
@@ -72,7 +72,7 @@ struct FEnv {
            ((status & FE_FLUSHTOZERO) ? EX_FLUSHTOZERO : 0);
   }
 
-  LIBC_INLINE static uint32_t exception_value_from_control(int control) {
+  LIBC_INLINE static uint32_t exception_value_from_control(uint32_t control) {
     return ((control & __fpcr_trap_invalid) ? EX_INVALID : 0) |
            ((control & __fpcr_trap_divbyzero) ? EX_DIVBYZERO : 0) |
            ((control & __fpcr_trap_overflow) ? EX_OVERFLOW : 0) |
@@ -81,7 +81,7 @@ struct FEnv {
            ((control & __fpcr_flush_to_zero) ? EX_FLUSHTOZERO : 0);
   }
 
-  LIBC_INLINE static int exception_value_to_status(uint32_t excepts) {
+  LIBC_INLINE static uint32_t exception_value_to_status(uint32_t excepts) {
     return ((excepts & EX_INVALID) ? FE_INVALID : 0) |
            ((excepts & EX_DIVBYZERO) ? FE_DIVBYZERO : 0) |
            ((excepts & EX_OVERFLOW) ? FE_OVERFLOW : 0) |
@@ -90,7 +90,7 @@ struct FEnv {
            ((excepts & EX_FLUSHTOZERO) ? FE_FLUSHTOZERO : 0);
   }
 
-  LIBC_INLINE static int exception_value_to_control(uint32_t excepts) {
+  LIBC_INLINE static uint32_t exception_value_to_control(uint32_t excepts) {
     return ((excepts & EX_INVALID) ? __fpcr_trap_invalid : 0) |
            ((excepts & EX_DIVBYZERO) ? __fpcr_trap_divbyzero : 0) |
            ((excepts & EX_OVERFLOW) ? __fpcr_trap_overflow : 0) |
@@ -113,34 +113,37 @@ struct FEnv {
 };
 
 LIBC_INLINE int enable_except(int excepts) {
-  uint32_t new_excepts = FEnv::exception_value_from_status(excepts);
+  uint32_t new_excepts =
+      FEnv::exception_value_from_status(static_cast<uint32_t>(excepts));
   uint32_t control_word = FEnv::get_control_word();
   uint32_t old_excepts = FEnv::exception_value_from_control(control_word);
   if (new_excepts != old_excepts) {
     control_word |= FEnv::exception_value_to_control(new_excepts);
     FEnv::set_control_word(control_word);
   }
-  return FEnv::exception_value_to_status(old_excepts);
+  return static_cast<int>(FEnv::exception_value_to_status(old_excepts));
 }
 
 LIBC_INLINE int disable_except(int excepts) {
-  uint32_t disabled_excepts = FEnv::exception_value_from_status(excepts);
+  uint32_t disabled_excepts =
+      FEnv::exception_value_from_status(static_cast<uint32_t>(excepts));
   uint32_t control_word = FEnv::get_control_word();
   uint32_t old_excepts = FEnv::exception_value_from_control(control_word);
   control_word &= ~FEnv::exception_value_to_control(disabled_excepts);
   FEnv::set_control_word(control_word);
-  return FEnv::exception_value_to_status(old_excepts);
+  return static_cast<int>(FEnv::exception_value_to_status(old_excepts));
 }
 
 LIBC_INLINE int get_except() {
   uint32_t control_word = FEnv::get_control_word();
   uint32_t enabled_excepts = FEnv::exception_value_from_control(control_word);
-  return FEnv::exception_value_to_status(enabled_excepts);
+  return static_cast<int>(FEnv::exception_value_to_status(enabled_excepts));
 }
 
 LIBC_INLINE int clear_except(int excepts) {
   uint32_t status_word = FEnv::get_status_word();
-  uint32_t except_value = FEnv::exception_value_from_status(excepts);
+  uint32_t except_value =
+      FEnv::exception_value_from_status(static_cast<uint32_t>(excepts));
   status_word &= ~FEnv::exception_value_to_status(except_value);
   FEnv::set_status_word(status_word);
   return 0;
@@ -148,13 +151,16 @@ LIBC_INLINE int clear_except(int excepts) {
 
 LIBC_INLINE int test_except(int excepts) {
   uint32_t statusWord = FEnv::get_status_word();
-  uint32_t ex_value = FEnv::exception_value_from_status(excepts);
-  return statusWord & FEnv::exception_value_to_status(ex_value);
+  uint32_t ex_value =
+      FEnv::exception_value_from_status(static_cast<uint32_t>(excepts));
+  return static_cast<int>(statusWord &
+                          FEnv::exception_value_to_status(ex_value));
 }
 
 LIBC_INLINE int set_except(int excepts) {
   uint32_t status_word = FEnv::get_status_word();
-  uint32_t new_exceptions = FEnv::exception_value_from_status(excepts);
+  uint32_t new_exceptions =
+      FEnv::exception_value_from_status(static_cast<uint32_t>(excepts));
   status_word |= FEnv::exception_value_to_status(new_exceptions);
   FEnv::set_status_word(status_word);
   return 0;
@@ -174,7 +180,8 @@ LIBC_INLINE int raise_except(int excepts) {
                          : "s0", "s1" /* s0 and s1 are clobbered */);
   };
 
-  uint32_t to_raise = FEnv::exception_value_from_status(excepts);
+  uint32_t to_raise =
+      FEnv::exception_value_from_status(static_cast<uint32_t>(excepts));
   int result = 0;
 
   if (to_raise & FEnv::EX_INVALID) {
@@ -237,7 +244,7 @@ LIBC_INLINE int get_round() {
 }
 
 LIBC_INLINE int set_round(int mode) {
-  uint16_t bit_value;
+  uint32_t bit_value;
   switch (mode) {
   case FE_TONEAREST:
     bit_value = FEnv::TONEAREST;
@@ -256,7 +263,7 @@ LIBC_INLINE int set_round(int mode) {
   }
 
   uint32_t control_word = FEnv::get_control_word();
-  control_word &= ~(0x3 << FEnv::ROUNDING_CONTROL_BIT_POSITION);
+  control_word &= ~(0x3u << FEnv::ROUNDING_CONTROL_BIT_POSITION);
   control_word |= (bit_value << FEnv::ROUNDING_CONTROL_BIT_POSITION);
   FEnv::set_control_word(control_word);
 
diff --git a/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h b/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
index 9492d52da0455..0ba836d17a085 100644
--- a/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
+++ b/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
@@ -24,7 +24,7 @@ namespace x86 {
 LIBC_INLINE void normalize(int &exponent,
                            FPBits<long double>::StorageType &mantissa) {
   const unsigned int shift = static_cast<unsigned int>(
-      cpp::countl_zero(static_cast<uint64_t>(mantissa)) -
+      static_cast<size_t>(cpp::countl_zero(static_cast<uint64_t>(mantissa))) -
       (8 * sizeof(uint64_t) - 1 - FPBits<long double>::FRACTION_LEN));
   exponent -= shift;
   mantissa <<= shift;
diff --git a/libc/src/__support/OSUtil/darwin/io.h b/libc/src/__support/OSUtil/darwin/io.h
index a5f7ecbd70362..69df99da522fb 100644
--- a/libc/src/__support/OSUtil/darwin/io.h
+++ b/libc/src/__support/OSUtil/darwin/io.h
@@ -17,7 +17,8 @@ namespace LIBC_NAMESPACE_DECL {
 
 LIBC_INLINE void write_to_stderr(cpp::string_view msg) {
   LIBC_NAMESPACE::syscall_impl(4 /*SYS_write*/, 2 /* stderr */,
-                               reinterpret_cast<long>(msg.data()), msg.size());
+                               reinterpret_cast<long>(msg.data()),
+                               static_cast<long>(msg.size()));
 }
 
 } // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/__support/big_int.h b/libc/src/__support/big_int.h
index fb5ad99d53e7b..85a17f3110d6a 100644
--- a/libc/src/__support/big_int.h
+++ b/libc/src/__support/big_int.h
@@ -272,19 +272,19 @@ 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) -> int {
+  constexpr auto at = [](size_t index) -> size_t {
     // reverse iteration when direction == LEFT.
     if constexpr (direction == LEFT)
-      return int(N) - int(index) - 1;
-    return int(index);
+      return N - index - 1;
+    return index;
   };
   const auto safe_get_at = [&](size_t index) -> word {
     // return appropriate value when accessing out of bound elements.
-    const int i = at(index);
+    const size_t i = at(index);
     if (i < 0)
       return 0;
     if (i >= int(N))
-      return is_neg ? -1 : 0;
+      return is_neg ? cpp::numeric_limits<word>::max() : 0;
     return array[i];
   };
   const size_t index_offset = offset / WORD_BITS;
@@ -697,7 +697,8 @@ struct BigInt {
     }
     BigInt quotient;
     WordType x_word = static_cast<WordType>(x);
-    constexpr size_t LOG2_WORD_SIZE = cpp::bit_width(WORD_SIZE) - 1;
+    constexpr size_t LOG2_WORD_SIZE =
+        static_cast<size_t>(cpp::bit_width(WORD_SIZE) - 1);
     constexpr size_t HALF_WORD_SIZE = WORD_SIZE >> 1;
     constexpr WordType HALF_MASK = ((WordType(1) << HALF_WORD_SIZE) - 1);
     // lower = smallest multiple of WORD_SIZE that is >= e.
@@ -1009,12 +1010,12 @@ struct BigInt {
       BigInt subtractor = divider;
       int cur_bit = multiword::countl_zero(subtractor.val) -
                     multiword::countl_zero(remainder.val);
-      subtractor <<= cur_bit;
+      subtractor <<= static_cast<size_t>(cur_bit);
       for (; cur_bit >= 0 && remainder > 0; --cur_bit, subtractor >>= 1) {
         if (remainder < subtractor)
           continue;
         remainder -= subtractor;
-        quotient.set_bit(cur_bit);
+        quotient.set_bit(static_cast<size_t>(cur_bit));
       }
     }
     return Division{quotient, remainder};
@@ -1276,26 +1277,28 @@ rotr(T value, int rotate);
 template <typename T>
 [[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<is_big_int_v<T>, T>
 rotl(T value, int rotate) {
-  constexpr unsigned N = cpp::numeric_limits<T>::digits;
+  constexpr int N = cpp::numeric_limits<T>::digits;
   rotate = rotate % N;
   if (!rotate)
     return value;
   if (rotate < 0)
     return cpp::rotr<T>(value, -rotate);
-  return (value << rotate) | (value >> (N - rotate));
+  return (value << static_cast<size_t>(rotate)) |
+         (value >> (N - static_cast<size_t>(rotate)));
 }
 
 // Specialization of cpp::rotr ('bit.h') for BigInt.
 template <typename T>
 [[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<is_big_int_v<T>, T>
 rotr(T value, int rotate) {
-  constexpr unsigned N = cpp::numeric_limits<T>::digits;
+  constexpr int N = cpp::numeric_limits<T>::digits;
   rotate = rotate % N;
   if (!rotate)
     return value;
   if (rotate < 0)
     return cpp::rotl<T>(value, -rotate);
-  return (value >> rotate) | (value << (N - rotate));
+  return (value >> static_cast<size_t>(rotate)) |
+         (value << (N - static_cast<size_t>(rotate)));
 }
 
 } // namespace cpp
@@ -1312,7 +1315,7 @@ mask_trailing_ones() {
   T out; // zero initialized
   for (size_t i = 0; i <= QUOTIENT; ++i)
     out[i] = i < QUOTIENT
-                 ? -1
+                 ? cpp::numeric_limits<typename T::word_type>::max()
                  : mask_trailing_ones<typename T::word_type, REMAINDER>();
   return out;
 }
@@ -1328,7 +1331,7 @@ LIBC_INLINE constexpr cpp::enable_if_t<is_big_int_v<T>, T> mask_leading_ones() {
   T out; // zero initialized
   for (size_t i = QUOTIENT; i < T::WORD_COUNT; ++i)
     out[i] = i > QUOTIENT
-...
[truncated]

@lntue lntue self-requested a review February 28, 2025 15:06
@@ -48,7 +49,8 @@ template <typename T> class span {
using const_reference = const T &;
using iterator = T *;

LIBC_INLINE_VAR static constexpr size_type dynamic_extent = -1;
LIBC_INLINE_VAR static constexpr size_type dynamic_extent =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does adding static_cast<size_type>(-1) work, so that we don't need to add another dependency to this one?

@@ -36,8 +36,7 @@ function(_get_common_test_compile_options output_var c_test flags)
if(NOT LIBC_WNO_ERROR)
# list(APPEND compile_options "-Werror")
endif()
# list(APPEND compile_options "-Wconversion")
# list(APPEND compile_options "-Wno-sign-conversion")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is -Wno-sign-conversion removed and not enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, adding a -Wno-* is effectively disabling the warning, so I thought it best to remove the "disabled" warning.

Also, looks like -Wconversion also enables -Wsign-conversion, because I did see these errors as well.

I can add -Wsign-conversion here to be explicit about this warning as well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If -Wconversion also includes -Wsign-conversion then your changes look good to me. Thanks!

Copy link
Contributor Author

@vinay-deshmukh vinay-deshmukh Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I think it does not. Strange why I still saw it warn for some code. I'll fix it up in my next PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lntue lntue changed the title [libc] Enable -Wconversion [libc] Enable -Wconversion for tests. Feb 28, 2025
@lntue lntue merged commit 1e6e845 into llvm:main Mar 4, 2025
17 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 4, 2025

LLVM Buildbot has detected a new failure on builder libc-riscv32-qemu-yocto-fullbuild-dbg running on rv32gc-qemu-system while building libc at step 4 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/196/builds/5669

Here is the relevant piece of the build log for the reference
Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py ...' (failure)
...
[165/408] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.qsort.dir/qsort.cpp.o
[166/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.mempcpy.dir/mempcpy.cpp.o
[167/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strchrnul.dir/strchrnul.cpp.o
[168/408] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.quick_exit.dir/quick_exit.cpp.o
[169/408] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.qsort_r.dir/qsort_r.cpp.o
[170/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strchr.dir/strchr.cpp.o
[171/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strcat.dir/strcat.cpp.o
[172/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strcasestr.dir/strcasestr.cpp.o
[173/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strerror.dir/strerror.cpp.o
[174/408] Building CXX object libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.converter.dir/converter.cpp.o
FAILED: libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.converter.dir/converter.cpp.o 
/usr/local/bin/clang++ -DLIBC_NAMESPACE=__llvm_libc_20_0_0_git -D_DEBUG -I/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc -isystem /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/build/libc/include -mabi=ilp32d -march=rv32imafdc --target=riscv32-unknown-linux-gnu --sysroot=/opt/riscv/sysroot --gcc-toolchain=/opt/riscv -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -g -std=gnu++17 --target=riscv32-unknown-linux-gnu -DLIBC_QSORT_IMPL=LIBC_QSORT_QUICK_SORT -DLIBC_ADD_NULL_CHECKS -fpie -ffreestanding -DLIBC_FULL_BUILD -nostdlibinc -idirafter=/usr/include -ffixed-point -fno-builtin -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -Wall -Wextra -Werror -Wconversion -Wno-sign-conversion -Wdeprecated -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety -Wglobal-constructors -DLIBC_COPT_RAW_MUTEX_DEFAULT_SPIN_COUNT=100 -DLIBC_COPT_TIMEOUT_ENSURE_MONOTONICITY=1 -MD -MT libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.converter.dir/converter.cpp.o -MF libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.converter.dir/converter.cpp.o.d -o libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.converter.dir/converter.cpp.o -c /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/stdio/scanf_core/converter.cpp
In file included from /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/stdio/scanf_core/converter.cpp:19:
In file included from /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/stdio/scanf_core/current_pos_converter.h:13:
In file included from /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/stdio/scanf_core/converter_utils.h:14:
In file included from /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/str_to_float.h:27:
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/high_precision_decimal.h:428:12: error: no matching conversion for static_cast from 'bool' to '__llvm_libc_20_0_0_git::BigInt<128, false>'
  428 |            static_cast<T>(this->should_round_up(this->decimal_point, round));
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/str_to_float.h:417:36: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::HighPrecisionDecimal::round_to_integer_type<__llvm_libc_20_0_0_git::BigInt<128, false>>' requested here
  417 |   StorageType final_mantissa = hpd.round_to_integer_type<StorageType>();
      |                                    ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/str_to_float.h:748:12: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::simple_decimal_conversion<long double>' requested here
  748 |   output = simple_decimal_conversion<T>(numStart, num_len, round);
      |            ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/str_to_float.h:982:9: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::decimal_exp_to_float<long double>' requested here
  982 |         decimal_exp_to_float<T>({mantissa, exponent}, truncated, round, src);
      |         ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/str_to_float.h:1191:22: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::decimal_string_to_float<long double>' requested here
 1191 |       parse_result = decimal_string_to_float<T>(src + index, DECIMAL_POINT,
      |                      ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/stdio/scanf_core/converter_utils.h:87:28: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::strtofloatingpoint<long double>' requested here
   87 |     auto value = internal::strtofloatingpoint<long double>(str);
      |                            ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/big_int.h:362:25: note: candidate constructor not viable: no known conversion from 'bool' to 'const BigInt<128, false>' for 1st argument
  362 |   LIBC_INLINE constexpr BigInt(const BigInt &other) = default;
      |                         ^      ~~~~~~~~~~~~~~~~~~~
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/big_int.h:462:34: note: candidate constructor not viable: no known conversion from 'bool' to 'const cpp::array<unsigned long long, WORD_COUNT>' for 1st argument
  462 |   LIBC_INLINE constexpr explicit BigInt(
      |                                  ^
  463 |       const cpp::array<WordType, WORD_COUNT> &words) {
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/big_int.h:365:25: note: candidate template ignored: could not match 'BigInt<OtherBits, OtherSigned, OtherWordType>' against 'bool'
  365 |   LIBC_INLINE constexpr BigInt(
      |                         ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/big_int.h:456:45: note: candidate template ignored: could not match 'unsigned long long[N]' against 'bool'
  456 |   template <size_t N> LIBC_INLINE constexpr BigInt(const WordType (&nums)[N]) {
      |                                             ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/big_int.h:470:25: note: candidate template ignored: substitution failure [with T = bool]: implicit instantiation of undefined template '__llvm_libc_20_0_0_git::cpp::enable_if<false>'
Step 6 (build libc) failure: build libc (failure)
...
[165/408] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.qsort.dir/qsort.cpp.o
[166/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.mempcpy.dir/mempcpy.cpp.o
[167/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strchrnul.dir/strchrnul.cpp.o
[168/408] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.quick_exit.dir/quick_exit.cpp.o
[169/408] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.qsort_r.dir/qsort_r.cpp.o
[170/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strchr.dir/strchr.cpp.o
[171/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strcat.dir/strcat.cpp.o
[172/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strcasestr.dir/strcasestr.cpp.o
[173/408] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strerror.dir/strerror.cpp.o
[174/408] Building CXX object libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.converter.dir/converter.cpp.o
FAILED: libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.converter.dir/converter.cpp.o 
/usr/local/bin/clang++ -DLIBC_NAMESPACE=__llvm_libc_20_0_0_git -D_DEBUG -I/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc -isystem /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/build/libc/include -mabi=ilp32d -march=rv32imafdc --target=riscv32-unknown-linux-gnu --sysroot=/opt/riscv/sysroot --gcc-toolchain=/opt/riscv -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -g -std=gnu++17 --target=riscv32-unknown-linux-gnu -DLIBC_QSORT_IMPL=LIBC_QSORT_QUICK_SORT -DLIBC_ADD_NULL_CHECKS -fpie -ffreestanding -DLIBC_FULL_BUILD -nostdlibinc -idirafter=/usr/include -ffixed-point -fno-builtin -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -Wall -Wextra -Werror -Wconversion -Wno-sign-conversion -Wdeprecated -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety -Wglobal-constructors -DLIBC_COPT_RAW_MUTEX_DEFAULT_SPIN_COUNT=100 -DLIBC_COPT_TIMEOUT_ENSURE_MONOTONICITY=1 -MD -MT libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.converter.dir/converter.cpp.o -MF libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.converter.dir/converter.cpp.o.d -o libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.converter.dir/converter.cpp.o -c /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/stdio/scanf_core/converter.cpp
In file included from /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/stdio/scanf_core/converter.cpp:19:
In file included from /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/stdio/scanf_core/current_pos_converter.h:13:
In file included from /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/stdio/scanf_core/converter_utils.h:14:
In file included from /home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/str_to_float.h:27:
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/high_precision_decimal.h:428:12: error: no matching conversion for static_cast from 'bool' to '__llvm_libc_20_0_0_git::BigInt<128, false>'
  428 |            static_cast<T>(this->should_round_up(this->decimal_point, round));
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/str_to_float.h:417:36: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::HighPrecisionDecimal::round_to_integer_type<__llvm_libc_20_0_0_git::BigInt<128, false>>' requested here
  417 |   StorageType final_mantissa = hpd.round_to_integer_type<StorageType>();
      |                                    ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/str_to_float.h:748:12: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::simple_decimal_conversion<long double>' requested here
  748 |   output = simple_decimal_conversion<T>(numStart, num_len, round);
      |            ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/str_to_float.h:982:9: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::decimal_exp_to_float<long double>' requested here
  982 |         decimal_exp_to_float<T>({mantissa, exponent}, truncated, round, src);
      |         ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/str_to_float.h:1191:22: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::decimal_string_to_float<long double>' requested here
 1191 |       parse_result = decimal_string_to_float<T>(src + index, DECIMAL_POINT,
      |                      ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/stdio/scanf_core/converter_utils.h:87:28: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::strtofloatingpoint<long double>' requested here
   87 |     auto value = internal::strtofloatingpoint<long double>(str);
      |                            ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/big_int.h:362:25: note: candidate constructor not viable: no known conversion from 'bool' to 'const BigInt<128, false>' for 1st argument
  362 |   LIBC_INLINE constexpr BigInt(const BigInt &other) = default;
      |                         ^      ~~~~~~~~~~~~~~~~~~~
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/big_int.h:462:34: note: candidate constructor not viable: no known conversion from 'bool' to 'const cpp::array<unsigned long long, WORD_COUNT>' for 1st argument
  462 |   LIBC_INLINE constexpr explicit BigInt(
      |                                  ^
  463 |       const cpp::array<WordType, WORD_COUNT> &words) {
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/big_int.h:365:25: note: candidate template ignored: could not match 'BigInt<OtherBits, OtherSigned, OtherWordType>' against 'bool'
  365 |   LIBC_INLINE constexpr BigInt(
      |                         ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/big_int.h:456:45: note: candidate template ignored: could not match 'unsigned long long[N]' against 'bool'
  456 |   template <size_t N> LIBC_INLINE constexpr BigInt(const WordType (&nums)[N]) {
      |                                             ^
/home/libcrv32buildbot/bbroot/libc-riscv32-qemu-yocto-fullbuild-dbg/llvm-project/libc/src/__support/big_int.h:470:25: note: candidate template ignored: substitution failure [with T = bool]: implicit instantiation of undefined template '__llvm_libc_20_0_0_git::cpp::enable_if<false>'

@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 4, 2025

LLVM Buildbot has detected a new failure on builder libc-arm32-debian-dbg running on libc-arm32-debian while building libc at step 4 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/182/builds/8771

Here is the relevant piece of the build log for the reference
Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py ...' (failure)
...
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/UnitTest/Test.h:37:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/UnitTest/LibcTest.h:29:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/CPP/string.h:18:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/string/memory_utils/inline_memcpy.h:32:
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/string/memory_utils/generic/byte_per_byte.h:41:35: warning: implicit conversion changes signedness: 'unsigned int' to 'ptrdiff_t' (aka 'int') [-Wsign-conversion]
    for (ptrdiff_t offset = count - 1; offset >= 0; --offset)
                   ~~~~~~   ~~~~~~^~~
1 warning generated.
[436/2222] Linking CXX executable libc/test/src/__support/libc.test.src.__support.blockstore_test.__unit__.__build__
[437/2222] Building CXX object libc/test/src/__support/CMakeFiles/libc.test.src.__support.high_precision_decimal_test.__unit__.__build__.dir/high_precision_decimal_test.cpp.o
FAILED: libc/test/src/__support/CMakeFiles/libc.test.src.__support.high_precision_decimal_test.__unit__.__build__.dir/high_precision_decimal_test.cpp.o 
/llvm/llvm-install/bin/clang++ -DLIBC_NAMESPACE=__llvm_libc_20_0_0_git -D_DEBUG -I/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc -isystem /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/build/libc/include -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -fdiagnostics-color -g -fpie -Wconversion -Wimplicit-fallthrough -Wwrite-strings -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wstrict-prototypes -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wthread-safety -std=gnu++17 -MD -MT libc/test/src/__support/CMakeFiles/libc.test.src.__support.high_precision_decimal_test.__unit__.__build__.dir/high_precision_decimal_test.cpp.o -MF libc/test/src/__support/CMakeFiles/libc.test.src.__support.high_precision_decimal_test.__unit__.__build__.dir/high_precision_decimal_test.cpp.o.d -o libc/test/src/__support/CMakeFiles/libc.test.src.__support.high_precision_decimal_test.__unit__.__build__.dir/high_precision_decimal_test.cpp.o -c /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/src/__support/high_precision_decimal_test.cpp
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/src/__support/high_precision_decimal_test.cpp:12:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/UnitTest/Test.h:37:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/UnitTest/LibcTest.h:29:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/CPP/string.h:18:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/string/memory_utils/inline_memcpy.h:32:
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/string/memory_utils/generic/byte_per_byte.h:41:35: warning: implicit conversion changes signedness: 'unsigned int' to 'ptrdiff_t' (aka 'int') [-Wsign-conversion]
    for (ptrdiff_t offset = count - 1; offset >= 0; --offset)
                   ~~~~~~   ~~~~~~^~~
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/src/__support/high_precision_decimal_test.cpp:9:
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/high_precision_decimal.h:428:12: error: no matching conversion for static_cast from 'bool' to '__llvm_libc_20_0_0_git::BigInt<128, false, unsigned long long>'
           static_cast<T>(this->should_round_up(this->decimal_point, round));
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/src/__support/high_precision_decimal_test.cpp:348:17: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::HighPrecisionDecimal::round_to_integer_type<__llvm_libc_20_0_0_git::BigInt<128, false, unsigned long long>>' requested here
  EXPECT_EQ(hpd.round_to_integer_type<UInt128>(), UInt128(1));
                ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:362:25: note: candidate constructor not viable: no known conversion from 'bool' to 'const __llvm_libc_20_0_0_git::BigInt<128, false, unsigned long long>' for 1st argument
  LIBC_INLINE constexpr BigInt(const BigInt &other) = default;
                        ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:462:34: note: candidate constructor not viable: no known conversion from 'bool' to 'const cpp::array<unsigned long long, WORD_COUNT>' for 1st argument
  LIBC_INLINE constexpr explicit BigInt(
                                 ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:365:25: note: candidate template ignored: could not match 'BigInt<Bits, Signed, type-parameter-0-2>' against 'bool'
  LIBC_INLINE constexpr BigInt(
                        ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:456:45: note: candidate template ignored: could not match 'const unsigned long long [N]' against 'bool'
  template <size_t N> LIBC_INLINE constexpr BigInt(const WordType (&nums)[N]) {
                                            ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:470:25: note: candidate template ignored: substitution failure [with T = bool]: implicit instantiation of undefined template '__llvm_libc_20_0_0_git::cpp::enable_if<false, void>'
  LIBC_INLINE constexpr BigInt(T v) {
                        ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:360:25: note: candidate constructor not viable: requires 0 arguments, but 1 was provided
  LIBC_INLINE constexpr BigInt() = default;
                        ^
1 warning and 1 error generated.
[438/2222] Building CXX object libc/test/src/__support/CMakeFiles/libc.test.src.__support.math_extras_test.__unit__.__build__.dir/math_extras_test.cpp.o
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/src/__support/math_extras_test.cpp:13:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/UnitTest/Test.h:37:
Step 7 (libc-unit-tests) failure: libc-unit-tests (failure)
...
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/UnitTest/Test.h:37:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/UnitTest/LibcTest.h:29:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/CPP/string.h:18:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/string/memory_utils/inline_memcpy.h:32:
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/string/memory_utils/generic/byte_per_byte.h:41:35: warning: implicit conversion changes signedness: 'unsigned int' to 'ptrdiff_t' (aka 'int') [-Wsign-conversion]
    for (ptrdiff_t offset = count - 1; offset >= 0; --offset)
                   ~~~~~~   ~~~~~~^~~
1 warning generated.
[436/2222] Linking CXX executable libc/test/src/__support/libc.test.src.__support.blockstore_test.__unit__.__build__
[437/2222] Building CXX object libc/test/src/__support/CMakeFiles/libc.test.src.__support.high_precision_decimal_test.__unit__.__build__.dir/high_precision_decimal_test.cpp.o
FAILED: libc/test/src/__support/CMakeFiles/libc.test.src.__support.high_precision_decimal_test.__unit__.__build__.dir/high_precision_decimal_test.cpp.o 
/llvm/llvm-install/bin/clang++ -DLIBC_NAMESPACE=__llvm_libc_20_0_0_git -D_DEBUG -I/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc -isystem /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/build/libc/include -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -fdiagnostics-color -g -fpie -Wconversion -Wimplicit-fallthrough -Wwrite-strings -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wstrict-prototypes -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wthread-safety -std=gnu++17 -MD -MT libc/test/src/__support/CMakeFiles/libc.test.src.__support.high_precision_decimal_test.__unit__.__build__.dir/high_precision_decimal_test.cpp.o -MF libc/test/src/__support/CMakeFiles/libc.test.src.__support.high_precision_decimal_test.__unit__.__build__.dir/high_precision_decimal_test.cpp.o.d -o libc/test/src/__support/CMakeFiles/libc.test.src.__support.high_precision_decimal_test.__unit__.__build__.dir/high_precision_decimal_test.cpp.o -c /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/src/__support/high_precision_decimal_test.cpp
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/src/__support/high_precision_decimal_test.cpp:12:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/UnitTest/Test.h:37:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/UnitTest/LibcTest.h:29:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/CPP/string.h:18:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/string/memory_utils/inline_memcpy.h:32:
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/string/memory_utils/generic/byte_per_byte.h:41:35: warning: implicit conversion changes signedness: 'unsigned int' to 'ptrdiff_t' (aka 'int') [-Wsign-conversion]
    for (ptrdiff_t offset = count - 1; offset >= 0; --offset)
                   ~~~~~~   ~~~~~~^~~
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/src/__support/high_precision_decimal_test.cpp:9:
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/high_precision_decimal.h:428:12: error: no matching conversion for static_cast from 'bool' to '__llvm_libc_20_0_0_git::BigInt<128, false, unsigned long long>'
           static_cast<T>(this->should_round_up(this->decimal_point, round));
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/src/__support/high_precision_decimal_test.cpp:348:17: note: in instantiation of function template specialization '__llvm_libc_20_0_0_git::internal::HighPrecisionDecimal::round_to_integer_type<__llvm_libc_20_0_0_git::BigInt<128, false, unsigned long long>>' requested here
  EXPECT_EQ(hpd.round_to_integer_type<UInt128>(), UInt128(1));
                ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:362:25: note: candidate constructor not viable: no known conversion from 'bool' to 'const __llvm_libc_20_0_0_git::BigInt<128, false, unsigned long long>' for 1st argument
  LIBC_INLINE constexpr BigInt(const BigInt &other) = default;
                        ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:462:34: note: candidate constructor not viable: no known conversion from 'bool' to 'const cpp::array<unsigned long long, WORD_COUNT>' for 1st argument
  LIBC_INLINE constexpr explicit BigInt(
                                 ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:365:25: note: candidate template ignored: could not match 'BigInt<Bits, Signed, type-parameter-0-2>' against 'bool'
  LIBC_INLINE constexpr BigInt(
                        ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:456:45: note: candidate template ignored: could not match 'const unsigned long long [N]' against 'bool'
  template <size_t N> LIBC_INLINE constexpr BigInt(const WordType (&nums)[N]) {
                                            ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:470:25: note: candidate template ignored: substitution failure [with T = bool]: implicit instantiation of undefined template '__llvm_libc_20_0_0_git::cpp::enable_if<false, void>'
  LIBC_INLINE constexpr BigInt(T v) {
                        ^
/llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/src/__support/big_int.h:360:25: note: candidate constructor not viable: requires 0 arguments, but 1 was provided
  LIBC_INLINE constexpr BigInt() = default;
                        ^
1 warning and 1 error generated.
[438/2222] Building CXX object libc/test/src/__support/CMakeFiles/libc.test.src.__support.math_extras_test.__unit__.__build__.dir/math_extras_test.cpp.o
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/src/__support/math_extras_test.cpp:13:
In file included from /llvm/libc_worker/worker/libc-arm32-debian/libc-arm32-debian-dbg/llvm-project/libc/test/UnitTest/Test.h:37:

michaelrj-google added a commit to michaelrj-google/llvm-project that referenced this pull request Mar 4, 2025
Followup to llvm#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.
michaelrj-google added a commit that referenced this pull request Mar 4, 2025
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.
@@ -293,7 +293,7 @@ LIBC_INLINE void store64_aligned(uint64_t value, Ptr dst, size_t offset) {
// Advances the pointers p1 and p2 by offset bytes and decrease count by the
// same amount.
template <typename T1, typename T2>
LIBC_INLINE void adjust(ptrdiff_t offset, T1 *__restrict &p1,
LIBC_INLINE void adjust(uintptr_t offset, T1 *__restrict &p1,
Copy link
Contributor

@Caslyn Caslyn Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello - there's a call to adjust that passes a negative value here: https://github.com/llvm/llvm-project/blob/main/libc/src/string/memory_utils/op_generic.h#L270. With this code change, this can raise an overflow error on asan builds. Could you take a second look at the callers of this function and make adjustments (or revert if this can't be resolved quickly)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, AFAIK ptrdiff_t is the only correct type here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.

I (incorrectly) concluded that adding to count was malformed, probably due to mixing of ptrdiff_t and size_t

Strange that the warnings didn't catch the negative argument

Will fix it

durin42 added a commit to durin42/llvm-project that referenced this pull request Mar 5, 2025
This reverts commit 1e6e845 because it
changed the 1st parameter of adjust() to be unsigned, but libc itself
calls adjust() with a negative argument in align_backward() in
op_generic.h.
durin42 added a commit to durin42/llvm-project that referenced this pull request Mar 5, 2025
This reverts commit 1e6e845 because it
changed the 1st parameter of adjust() to be unsigned, but libc itself
calls adjust() with a negative argument in align_backward() in
op_generic.h.
@durin42
Copy link
Contributor

durin42 commented Mar 5, 2025

I rolled this back for now because libc has a call to one of these functions that breaks under ubsan - see da61b0d for details.

@lntue graciously offered to try and fix-forward but said it wasn't feasible quickly, but hopefully this is enough information for you to redo this easily with the needed fixes. :)

vinay-deshmukh added a commit to vinay-deshmukh/llvm-project that referenced this pull request Mar 6, 2025
vinay-deshmukh added a commit to vinay-deshmukh/llvm-project that referenced this pull request Mar 6, 2025
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
Followup to llvm#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.
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
This reverts commit 1e6e845 because it
changed the 1st parameter of adjust() to be unsigned, but libc itself
calls adjust() with a negative argument in align_backward() in
op_generic.h.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants