[libc] Add -Wno-sign-conversion & re-attempt -Wconversion
#129811
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 5, 2025
|
|
vinay-deshmukh
commented
Mar 5, 2025
libc/src/__support/big_int.h
Outdated
| @@ -285,7 +285,10 @@ LIBC_INLINE constexpr cpp::array<word, N> shift(cpp::array<word, N> array, | |||
| return 0; | |||
| if (i >= int(N)) | |||
| return is_neg ? cpp::numeric_limits<word>::max() : 0; | |||
| #pragma GCC diagnostic push | |||
| #pragma GCC diagnostic ignored "-Wsign-conversion" | |||
| return array[i]; | |||
There was a problem hiding this comment.
This is the only code that get flagged (atleast locally for an overlay build).
it was changed to use size_t in: #127523
but had to be reverted in: #129771 (comment)
Discussion on why it needs signed int has begun here: #129771 (comment)
There was a problem hiding this comment.
Note: pragma(s) are only WIP for now, will be fixed in final version of PR based on above discussion
There was a problem hiding this comment.
I think this would be a simpler solution:
Suggested change
| return array[i]; | |
| return static_cast<word>(array[i]); |
There was a problem hiding this comment.
This didn't work, as the error is at:
llvm-project/libc/src/__support/big_int.h:288:36: error: implicit conversion changes signedness: 'const int' to 'size_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
return static_cast<word>(array[i]);
But:
Suggested change
| return array[i]; | |
| return array[static_cast<size_t>(i)]; |
did work
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
vinay-deshmukh
force-pushed
the
vinay-issue-119281-sign-conversion
branch
from
0d17ad6 to
958813a
Compare
This reverts commit da61b0d.
vinay-deshmukh
changed the title
-Wsign-conversion-Wsign-conversion & un-reverts -Wconversion
vinay-deshmukh
commented
Mar 6, 2025
| # list(APPEND compile_options "-Wconversion") | ||
| # list(APPEND compile_options "-Wno-sign-conversion") | ||
| list(APPEND compile_options "-Wconversion") | ||
| list(APPEND compile_options "-Wsign-conversion") |
There was a problem hiding this comment.
Because -Wno-sign-conversion is effectively "silencing" it
There was a problem hiding this comment.
I tried this PR and there are still many warnings from -Wsign-conversion:
$ ninja check-libc &> test.txt
$ grep -c "Wsign-conversion" test.txt
12651
Some example:
In file included from /usr/local/google/home/lntue/experiment/llvm-project/libc/src/__support/FPUtil/FEnvImpl.h:31:
/usr/local/google/home/lntue/experiment/llvm-project/libc/src/__support/FPUtil/x86_64/FEnvImpl.h:169:12: warning: implicit conversion changes signedness: 'int' to 'uint32_t' (aka 'unsigned int') [-Wsign-conversion]
mxcsr &= ~(bit_mask << internal::MXCSR_EXCEPTION_CONTOL_BIT_POISTION);
~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/lntue/experiment/llvm-project/libc/src/__support/FPUtil/x86_64/FEnvImpl.h:193:22: warning: implicit conversion changes signedness: 'int' to 'uint32_t' (aka 'unsigned int') [-Wsign-conversion]
mxcsr |= (bit_mask << internal::MXCSR_EXCEPTION_CONTOL_BIT_POISTION);
~~ ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/lntue/experiment/llvm-project/libc/src/__support/FPUtil/x86_64/FEnvImpl.h:341:26: warning: implicit conversion changes signedness: 'int' to 'uint32_t' (aka 'unsigned int') [-Wsign-conversion]
<< internal::MXCSR_ROUNDING_CONTROL_BIT_POSITION;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/lntue/experiment/llvm-project/libc/src/__support/FPUtil/x86_64/FEnvImpl.h:344:20: warning: implicit conversion changes signedness: 'int' to 'uint32_t' (aka 'unsigned int') [-Wsign-conversion]
~(0x3 << internal::MXCSR_ROUNDING_CONTROL_BIT_POSITION)) |
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/lntue/experiment/llvm-project/libc/src/__support/FPUtil/x86_64/FEnvImpl.h:610:14: warning: implicit conversion changes signedness: 'int' to 'uint32_t' (aka 'unsigned int') [-Wsign-conversion]
mxcsr &= ~uint16_t(0x3F); // Clear exception flags.
~~ ^~~~~~~~~~~~~~~
/usr/local/google/home/lntue/experiment/llvm-project/libc/src/__support/FPUtil/x86_64/FEnvImpl.h:611:14: warning: implicit conversion changes signedness: 'int' to 'uint32_t' (aka 'unsigned int') [-Wsign-conversion]
mxcsr &= ~(uint16_t(0x1) << 6); // Reset denormals-are-zero
~~ ^~~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/lntue/experiment/llvm-project/libc/src/__support/FPUtil/x86_64/FEnvImpl.h:613:14: warning: implicit conversion changes signedness: 'int' to 'uint32_t' (aka 'unsigned int') [-Wsign-conversion]
mxcsr &= ~(uint16_t(0x3) << 13); // Round to nearest.
~~ ^~~~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/lntue/experiment/llvm-project/libc/src/__support/FPUtil/x86_64/FEnvImpl.h:614:14: warning: implicit conversion changes signedness: 'int' to 'uint32_t' (aka 'unsigned int') [-Wsign-conversion]
mxcsr &= ~(uint16_t(0x1) << 15); // Reset flush-to-zero
~~ ^~~~~~~~~~~~~~~~~~~~~~
Probably you should change to -Wno-sign-conversion in this PR, add a TODO comment to clean it up, and then fix the remaining -Wsign-conversion in a followup PR.
There was a problem hiding this comment.
Is there a way to reproduce these in the PR checks?
I don't have a x86 64 machine at the moment
There was a problem hiding this comment.
You can see them in the logs of some of the premerge bots, such as:
https://buildkite.com/llvm-project/github-pull-requests/builds/154789#0195738e-55d8-4974-b5b7-1c857402d13d
https://github.com/llvm/llvm-project/actions/runs/13732988495/job/38413066727?pr=129811
There was a problem hiding this comment.
Made it -Wno-sign-conversion
|
@llvm/pr-subscribers-libc Author: Vinay Deshmukh (vinay-deshmukh) ChangesRelates to #119281 (comment) Patch is 72.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/129811.diff 47 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 03b4b251649e7..d332d5d044045 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -36,8 +36,8 @@ 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 "-Wsign-conversion")
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 e726a094b5dac..04e9f3cf656a1 100644
--- a/libc/src/__support/big_int.h
+++ b/libc/src/__support/big_int.h
@@ -284,8 +284,8 @@ LIBC_INLINE constexpr cpp::array<word, N> shift(cpp::array<word, N> array,
if (i < 0)
return 0;
if (i >= int(N))
- return is_neg ? -1 : 0;
- return array[i];
+ return is_neg ? cpp::numeric_limits<word>::max() : 0;
+ return array[static_cast<size_t>(i)];
};
const size_t index_offset = offset / WORD_BITS;
const size_t bit_offset = offset % WORD_BITS;
@@ -296,7 +296,7 @@ LIBC_INLINE constexpr cpp::array<word, N> shift(cpp::array<word, N> array,
for (size_t index = 0; index < N; ++index) {
const word part1 = safe_get_at(index + index_offset);
const word part2 = safe_get_at(index + index_offset + 1);
- word &dst = out[at(index)];
+ word &dst = out[static_cast<size_t>(at(index))];
if (bit_offset == 0)
dst = part1; // no crosstalk between parts.
else if constexpr (direction == LEFT)
@@ -696,7 +696,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.
@@ -1008,12 +1009,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};
@@ -1275,26 +1276,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
@@ -1311,7 +1314,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;
}
@@ -1327,7 +1330,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 > ...
[truncated]
|
vinay-deshmukh
commented
Mar 7, 2025
vinay-deshmukh
commented
Mar 7, 2025
vinay-deshmukh
commented
Mar 7, 2025
libc/src/__support/big_int.h
Outdated
| @@ -296,7 +296,7 @@ LIBC_INLINE constexpr cpp::array<word, N> shift(cpp::array<word, N> array, | |||
| for (size_t index = 0; index < N; ++index) { | |||
| const word part1 = safe_get_at(index + index_offset); | |||
| const word part2 = safe_get_at(index + index_offset + 1); | |||
| word &dst = out[at(index)]; | |||
| word &dst = out[static_cast<size_t>(at(index))]; | |||
There was a problem hiding this comment.
decltype(at(...)) is int, but operator[]needssize_t`
There was a problem hiding this comment.
does unsigned instead of size_t work for indexing?
There was a problem hiding this comment.
Looks like it works without raising a warning.
May I know what the difference would be here? AFAIK casting doesn't really change the type here (i guess) but only silences the warning? and unsigned would still be auto-promoted to size_t eventually?
There was a problem hiding this comment.
in general, 32-bit indexing would be enough, and unsigned is to prevent negative index.
There was a problem hiding this comment.
Used unsigned
|
Can you sync and resolve the merge conflicts? Thanks |
vinay-deshmukh
force-pushed
the
vinay-issue-119281-sign-conversion
branch
from
20ddbe9 to
2a05303
Compare
vinay-deshmukh
changed the title
-Wsign-conversion & un-reverts -Wconversion-Wno-sign-conversion & re-attempt -Wconversion
lntue
approved these changes
Mar 10, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/200/builds/5072 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/25248 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates to #119281 (comment)