[libc] [Task] Prepare to enable disabled warnings #122835
Open
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
vinay-deshmukh
commented
Jan 14, 2025
libc/test/UnitTest/LibcTest.h
Outdated
| @@ -81,7 +81,7 @@ struct Message { | |||
| // A trivial object to catch the Message, this enables custom logging and | |||
| // returning from the test function, see LIBC_TEST_SCAFFOLDING_ below. | |||
| struct Failure { | |||
| void operator=(Message msg) {} | |||
| void operator=(Message /*msg*/) {} | |||
There was a problem hiding this comment.
Used this style to disable "unused-parameter".
let me know if LLVM prefers [[maybe_unused]] over this instead.
There was a problem hiding this comment.
I guess the llvm style guide doesn't say. So I'm ok with what you've chosen.
|
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
vinay-deshmukh
force-pushed
the
vinay-issue-119281
branch
from
efcbc14 to
f375ca5
Compare
vinay-deshmukh
changed the title
|
@llvm/pr-subscribers-libc Author: Vinay Deshmukh (vinay-deshmukh) ChangesResolves #119281
Patch is 20.99 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/122835.diff 19 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 8dcee1ec422464..bf6c1c51a5c23a 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -241,17 +241,18 @@ function(_get_common_test_compile_options output_var c_test flags)
list(APPEND compile_options "-ffixed-point")
endif()
- # list(APPEND compile_options "-Wall")
- # list(APPEND compile_options "-Wextra")
+ list(APPEND compile_options "-Wall")
+ list(APPEND compile_options "-Wextra")
# -DLIBC_WNO_ERROR=ON if you can't build cleanly with -Werror.
if(NOT LIBC_WNO_ERROR)
- # list(APPEND compile_options "-Werror")
+ list(APPEND compile_options "-Werror")
+ list(APPEND compile_options "-Wno-global-constructors")
endif()
- # list(APPEND compile_options "-Wconversion")
- # list(APPEND compile_options "-Wno-sign-conversion")
- # list(APPEND compile_options "-Wimplicit-fallthrough")
- # list(APPEND compile_options "-Wwrite-strings")
- # list(APPEND compile_options "-Wextra-semi")
+ list(APPEND compile_options "-Wconversion")
+ list(APPEND compile_options "-Wno-sign-conversion")
+ list(APPEND compile_options "-Wimplicit-fallthrough")
+ list(APPEND compile_options "-Wwrite-strings")
+ list(APPEND compile_options "-Wextra-semi")
# Silence this warning because _Complex is a part of C99.
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
if(NOT c_test)
@@ -262,13 +263,13 @@ function(_get_common_test_compile_options output_var c_test flags)
list(APPEND compile_options "-Wno-gnu-imaginary-constant")
endif()
list(APPEND compile_options "-Wno-pedantic")
- # if(NOT CMAKE_COMPILER_IS_GNUCXX)
- # list(APPEND compile_options "-Wnewline-eof")
- # list(APPEND compile_options "-Wnonportable-system-include-path")
- # list(APPEND compile_options "-Wstrict-prototypes")
- # list(APPEND compile_options "-Wthread-safety")
- # list(APPEND compile_options "-Wglobal-constructors")
- # endif()
+ if(NOT CMAKE_COMPILER_IS_GNUCXX)
+ list(APPEND compile_options "-Wnewline-eof")
+ list(APPEND compile_options "-Wnonportable-system-include-path")
+ list(APPEND compile_options "-Wstrict-prototypes")
+ list(APPEND compile_options "-Wthread-safety")
+ # list(APPEND compile_options "-Wglobal-constructors") # triggered in TEST_F implementation
+ endif()
endif()
set(${output_var} ${compile_options} PARENT_SCOPE)
endfunction()
diff --git a/libc/src/__support/CPP/bit.h b/libc/src/__support/CPP/bit.h
index adcd0472747d01..82b9eb51282625 100644
--- a/libc/src/__support/CPP/bit.h
+++ b/libc/src/__support/CPP/bit.h
@@ -232,7 +232,7 @@ rotl(T value, int rotate) {
return value;
if (rotate < 0)
return cpp::rotr<T>(value, -rotate);
- return (value << rotate) | (value >> (N - rotate));
+ return static_cast<T>((value << rotate) | (value >> (N - rotate)));
}
template <typename T>
@@ -244,7 +244,7 @@ rotr(T value, int rotate) {
return value;
if (rotate < 0)
return cpp::rotl<T>(value, -rotate);
- return (value >> rotate) | (value << (N - rotate));
+ return static_cast<T>((value >> rotate) | (value << (N - rotate)));
}
// TODO: Do we need this function at all? How is it different from
diff --git a/libc/src/string/memory_utils/utils.h b/libc/src/string/memory_utils/utils.h
index cae65bddd92f6e..eb44e856a1a718 100644
--- a/libc/src/string/memory_utils/utils.h
+++ b/libc/src/string/memory_utils/utils.h
@@ -263,7 +263,7 @@ LIBC_INLINE void store_aligned(ValueType value, Ptr dst) {
static_assert(sizeof(ValueType) >= (sizeof(T) + ... + sizeof(TS)));
constexpr size_t SHIFT = sizeof(T) * 8;
if constexpr (Endian::IS_LITTLE) {
- store<T>(assume_aligned<sizeof(T)>(dst), value & ~T(0));
+ store<T>(assume_aligned<sizeof(T)>(dst), static_cast<T>(value & ~T(0)));
if constexpr (sizeof...(TS) > 0)
store_aligned<ValueType, TS...>(value >> SHIFT, dst + sizeof(T));
} else if constexpr (Endian::IS_BIG) {
diff --git a/libc/test/UnitTest/LibcDeathTestExecutors.cpp b/libc/test/UnitTest/LibcDeathTestExecutors.cpp
index 943e2c23c5fde9..f2b048864b556c 100644
--- a/libc/test/UnitTest/LibcDeathTestExecutors.cpp
+++ b/libc/test/UnitTest/LibcDeathTestExecutors.cpp
@@ -22,7 +22,7 @@ namespace LIBC_NAMESPACE_DECL {
namespace testing {
bool Test::testProcessKilled(testutils::FunctionCaller *Func, int Signal,
- const char *LHSStr, const char *RHSStr,
+ const char *LHSStr, const char * /*RHSStr*/,
internal::Location Loc) {
testutils::ProcessStatus Result =
testutils::invoke_in_subprocess(Func, TIMEOUT_MS);
diff --git a/libc/test/UnitTest/LibcTest.h b/libc/test/UnitTest/LibcTest.h
index b4e3819ea958de..124748ba343270 100644
--- a/libc/test/UnitTest/LibcTest.h
+++ b/libc/test/UnitTest/LibcTest.h
@@ -81,7 +81,7 @@ struct Message {
// A trivial object to catch the Message, this enables custom logging and
// returning from the test function, see LIBC_TEST_SCAFFOLDING_ below.
struct Failure {
- void operator=(Message msg) {}
+ void operator=(Message /*msg*/) {}
};
struct RunContext {
diff --git a/libc/test/include/stdbit_stub.h b/libc/test/include/stdbit_stub.h
index 8a8e30e889d6d7..e73a2117d6b575 100644
--- a/libc/test/include/stdbit_stub.h
+++ b/libc/test/include/stdbit_stub.h
@@ -17,11 +17,11 @@
#include <stdbool.h> // bool in C
#define STDBIT_STUB_FUNCTION(FUNC_NAME, LEADING_VAL) \
- unsigned FUNC_NAME##_uc(unsigned char x) { return LEADING_VAL##AU; } \
- unsigned FUNC_NAME##_us(unsigned short x) { return LEADING_VAL##BU; } \
- unsigned FUNC_NAME##_ui(unsigned int x) { return LEADING_VAL##CU; } \
- unsigned FUNC_NAME##_ul(unsigned long x) { return LEADING_VAL##DU; } \
- unsigned FUNC_NAME##_ull(unsigned long long x) { return LEADING_VAL##EU; }
+ unsigned FUNC_NAME##_uc(unsigned char) { return LEADING_VAL##AU; } \
+ unsigned FUNC_NAME##_us(unsigned short) { return LEADING_VAL##BU; } \
+ unsigned FUNC_NAME##_ui(unsigned int) { return LEADING_VAL##CU; } \
+ unsigned FUNC_NAME##_ul(unsigned long) { return LEADING_VAL##DU; } \
+ unsigned FUNC_NAME##_ull(unsigned long long) { return LEADING_VAL##EU; }
__BEGIN_C_DECLS
@@ -36,24 +36,24 @@ STDBIT_STUB_FUNCTION(stdc_first_trailing_one, 0x1)
STDBIT_STUB_FUNCTION(stdc_count_zeros, 0x2)
STDBIT_STUB_FUNCTION(stdc_count_ones, 0x3)
-bool stdc_has_single_bit_uc(unsigned char x) { return false; }
-bool stdc_has_single_bit_us(unsigned short x) { return false; }
-bool stdc_has_single_bit_ui(unsigned x) { return false; }
-bool stdc_has_single_bit_ul(unsigned long x) { return false; }
-bool stdc_has_single_bit_ull(unsigned long long x) { return false; }
+bool stdc_has_single_bit_uc(unsigned char) { return false; }
+bool stdc_has_single_bit_us(unsigned short) { return false; }
+bool stdc_has_single_bit_ui(unsigned) { return false; }
+bool stdc_has_single_bit_ul(unsigned long) { return false; }
+bool stdc_has_single_bit_ull(unsigned long long) { return false; }
STDBIT_STUB_FUNCTION(stdc_bit_width, 0x4)
-unsigned char stdc_bit_floor_uc(unsigned char x) { return 0x5AU; }
-unsigned short stdc_bit_floor_us(unsigned short x) { return 0x5BU; }
-unsigned stdc_bit_floor_ui(unsigned x) { return 0x5CU; }
-unsigned long stdc_bit_floor_ul(unsigned long x) { return 0x5DUL; }
-unsigned long long stdc_bit_floor_ull(unsigned long long x) { return 0x5EULL; }
-
-unsigned char stdc_bit_ceil_uc(unsigned char x) { return 0x6AU; }
-unsigned short stdc_bit_ceil_us(unsigned short x) { return 0x6BU; }
-unsigned stdc_bit_ceil_ui(unsigned x) { return 0x6CU; }
-unsigned long stdc_bit_ceil_ul(unsigned long x) { return 0x6DUL; }
-unsigned long long stdc_bit_ceil_ull(unsigned long long x) { return 0x6EULL; }
+unsigned char stdc_bit_floor_uc(unsigned char) { return 0x5AU; }
+unsigned short stdc_bit_floor_us(unsigned short) { return 0x5BU; }
+unsigned stdc_bit_floor_ui(unsigned) { return 0x5CU; }
+unsigned long stdc_bit_floor_ul(unsigned long) { return 0x5DUL; }
+unsigned long long stdc_bit_floor_ull(unsigned long long) { return 0x5EULL; }
+
+unsigned char stdc_bit_ceil_uc(unsigned char) { return 0x6AU; }
+unsigned short stdc_bit_ceil_us(unsigned short) { return 0x6BU; }
+unsigned stdc_bit_ceil_ui(unsigned) { return 0x6CU; }
+unsigned long stdc_bit_ceil_ul(unsigned long) { return 0x6DUL; }
+unsigned long long stdc_bit_ceil_ull(unsigned long long) { return 0x6EULL; }
__END_C_DECLS
diff --git a/libc/test/src/__support/CPP/bit_test.cpp b/libc/test/src/__support/CPP/bit_test.cpp
index 9429b66ad1f981..9389e96642d1ef 100644
--- a/libc/test/src/__support/CPP/bit_test.cpp
+++ b/libc/test/src/__support/CPP/bit_test.cpp
@@ -41,7 +41,7 @@ TYPED_TEST(LlvmLibcBitTest, HasSingleBit, UnsignedTypes) {
constexpr auto LSB = T(1);
constexpr auto MSB = T(~(ALL_ONES >> 1));
for (T value = 1; value; value <<= 1) {
- auto two_bits_value = value | ((value <= MIDPOINT) ? MSB : LSB);
+ T two_bits_value = value | ((value <= MIDPOINT) ? MSB : LSB);
EXPECT_FALSE(has_single_bit<T>(two_bits_value));
}
}
diff --git a/libc/test/src/__support/CPP/integer_sequence_test.cpp b/libc/test/src/__support/CPP/integer_sequence_test.cpp
index fe378522b446b9..56e75f496255f5 100644
--- a/libc/test/src/__support/CPP/integer_sequence_test.cpp
+++ b/libc/test/src/__support/CPP/integer_sequence_test.cpp
@@ -23,7 +23,8 @@ TEST(LlvmLibcIntegerSequencetTest, Basic) {
(is_same_v<ULLSeq, make_integer_sequence<unsigned long long, 4>>));
}
-template <typename T, T... Ts> bool checkArray(integer_sequence<T, Ts...> seq) {
+template <typename T, T... Ts>
+bool checkArray(integer_sequence<T, Ts...> /*seq*/) {
T arr[sizeof...(Ts)]{Ts...};
for (T i = 0; i < static_cast<T>(sizeof...(Ts)); i++)
diff --git a/libc/test/src/__support/arg_list_test.cpp b/libc/test/src/__support/arg_list_test.cpp
index 79a715e9106870..57714050f05fca 100644
--- a/libc/test/src/__support/arg_list_test.cpp
+++ b/libc/test/src/__support/arg_list_test.cpp
@@ -72,9 +72,9 @@ long int check_primitives(int first, ...) {
count += args.next_var<unsigned long>();
count += args.next_var<long long>();
count += args.next_var<unsigned long long>();
- count += args.next_var<double>();
- count += args.next_var<double>();
- count += args.next_var<long double>();
+ count += static_cast<long>(args.next_var<double>());
+ count += static_cast<long>(args.next_var<double>());
+ count += static_cast<long>(args.next_var<long double>());
count += *args.next_var<int *>();
return count;
}
@@ -112,7 +112,8 @@ long int check_struct_type(int first, ...) {
S s = args.next_var<S>();
int last = args.next_var<int>();
- return s.c + s.s + s.i + s.l + s.f + s.d + last;
+ return s.c + s.s + s.i + s.l + static_cast<long>(s.f) +
+ static_cast<long>(s.d) + last;
}
TEST(LlvmLibcArgListTest, TestStructTypes) {
diff --git a/libc/test/src/__support/big_int_test.cpp b/libc/test/src/__support/big_int_test.cpp
index 2666ed978dad7a..68afecb17057d3 100644
--- a/libc/test/src/__support/big_int_test.cpp
+++ b/libc/test/src/__support/big_int_test.cpp
@@ -197,7 +197,7 @@ TYPED_TEST(LlvmLibcUIntClassTest, CountBits, Types) {
for (size_t i = 0; i < T::BITS; ++i) {
const auto l_one = T::all_ones() << i; // 0b111...000
const auto r_one = T::all_ones() >> i; // 0b000...111
- const int zeros = i;
+ const int zeros = static_cast<int>(i);
const int ones = T::BITS - zeros;
ASSERT_EQ(cpp::countr_one(r_one), ones);
ASSERT_EQ(cpp::countl_one(l_one), ones);
diff --git a/libc/test/src/__support/hash_test.cpp b/libc/test/src/__support/hash_test.cpp
index f23a43a3bc5e4d..94c884cc7fb17e 100644
--- a/libc/test/src/__support/hash_test.cpp
+++ b/libc/test/src/__support/hash_test.cpp
@@ -78,7 +78,7 @@ TEST(LlvmLibcHashTest, Avalanche) {
}
for (size_t i = 0; i < sz; ++i) {
for (size_t j = 0; j < 8; ++j) {
- uint8_t mask = 1 << j;
+ uint8_t mask = static_cast<uint8_t>(1 << j);
mem.data[i] ^= mask;
{
LIBC_NAMESPACE::internal::HashState state{0xabcdef1234567890};
diff --git a/libc/test/src/__support/math_extras_test.cpp b/libc/test/src/__support/math_extras_test.cpp
index 08c090017c1a19..887164cfb398df 100644
--- a/libc/test/src/__support/math_extras_test.cpp
+++ b/libc/test/src/__support/math_extras_test.cpp
@@ -73,27 +73,27 @@ TEST(LlvmLibcBlockMathExtrasTest, mask_trailing_ones) {
TYPED_TEST(LlvmLibcBitTest, FirstLeadingZero, UnsignedTypesNoBigInt) {
EXPECT_EQ(first_leading_zero<T>(cpp::numeric_limits<T>::max()), 0);
for (int i = 0U; i != cpp::numeric_limits<T>::digits; ++i)
- EXPECT_EQ(first_leading_zero<T>(~(T(1) << i)),
+ EXPECT_EQ(first_leading_zero<T>(static_cast<T>(~(T(1) << i))),
cpp::numeric_limits<T>::digits - i);
}
TYPED_TEST(LlvmLibcBitTest, FirstLeadingOne, UnsignedTypesNoBigInt) {
EXPECT_EQ(first_leading_one<T>(static_cast<T>(0)), 0);
for (int i = 0U; i != cpp::numeric_limits<T>::digits; ++i)
- EXPECT_EQ(first_leading_one<T>(T(1) << i),
+ EXPECT_EQ(first_leading_one<T>(static_cast<T>(T(1) << i)),
cpp::numeric_limits<T>::digits - i);
}
TYPED_TEST(LlvmLibcBitTest, FirstTrailingZero, UnsignedTypesNoBigInt) {
EXPECT_EQ(first_trailing_zero<T>(cpp::numeric_limits<T>::max()), 0);
for (int i = 0U; i != cpp::numeric_limits<T>::digits; ++i)
- EXPECT_EQ(first_trailing_zero<T>(~(T(1) << i)), i + 1);
+ EXPECT_EQ(first_trailing_zero<T>(static_cast<T>(~(T(1) << i))), i + 1);
}
TYPED_TEST(LlvmLibcBitTest, FirstTrailingOne, UnsignedTypesNoBigInt) {
EXPECT_EQ(first_trailing_one<T>(cpp::numeric_limits<T>::max()), 0);
for (int i = 0U; i != cpp::numeric_limits<T>::digits; ++i)
- EXPECT_EQ(first_trailing_one<T>(T(1) << i), i + 1);
+ EXPECT_EQ(first_trailing_one<T>(static_cast<T>(T(1) << i)), i + 1);
}
TYPED_TEST(LlvmLibcBitTest, CountZeros, UnsignedTypesNoBigInt) {
diff --git a/libc/test/src/math/FModTest.h b/libc/test/src/math/FModTest.h
index 32c009ab88286d..c32328584582fc 100644
--- a/libc/test/src/math/FModTest.h
+++ b/libc/test/src/math/FModTest.h
@@ -54,7 +54,7 @@ class FmodTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
// fmod (+inf, y) == aNaN plus invalid exception.
TEST_SPECIAL(inf, 3.0, aNaN, true, FE_INVALID);
- TEST_SPECIAL(inf, -1.1L, aNaN, true, FE_INVALID);
+ TEST_SPECIAL(inf, static_cast<T>(-1.1L), aNaN, true, FE_INVALID);
TEST_SPECIAL(inf, 0.0, aNaN, true, FE_INVALID);
TEST_SPECIAL(inf, neg_zero, aNaN, true, FE_INVALID);
TEST_SPECIAL(inf, min_denormal, aNaN, true, FE_INVALID);
@@ -65,7 +65,7 @@ class FmodTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
// fmod (-inf, y) == aNaN plus invalid exception.
TEST_SPECIAL(neg_inf, 3.0, aNaN, true, FE_INVALID);
- TEST_SPECIAL(neg_inf, -1.1L, aNaN, true, FE_INVALID);
+ TEST_SPECIAL(neg_inf, static_cast<T>(-1.1L), aNaN, true, FE_INVALID);
TEST_SPECIAL(neg_inf, 0.0, aNaN, true, FE_INVALID);
TEST_SPECIAL(neg_inf, neg_zero, aNaN, true, FE_INVALID);
TEST_SPECIAL(neg_inf, min_denormal, aNaN, true, FE_INVALID);
@@ -76,7 +76,7 @@ class FmodTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
// fmod (x, +0) == aNaN plus invalid exception.
TEST_SPECIAL(3.0, 0.0, aNaN, true, FE_INVALID);
- TEST_SPECIAL(-1.1L, 0.0, aNaN, true, FE_INVALID);
+ TEST_SPECIAL(static_cast<T>(-1.1L), 0.0, aNaN, true, FE_INVALID);
TEST_SPECIAL(0.0, 0.0, aNaN, true, FE_INVALID);
TEST_SPECIAL(neg_zero, 0.0, aNaN, true, FE_INVALID);
TEST_SPECIAL(min_denormal, 0.0, aNaN, true, FE_INVALID);
@@ -85,7 +85,7 @@ class FmodTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
// fmod (x, -0) == aNaN plus invalid exception.
TEST_SPECIAL(3.0, neg_zero, aNaN, true, FE_INVALID);
- TEST_SPECIAL(-1.1L, neg_zero, aNaN, true, FE_INVALID);
+ TEST_SPECIAL(static_cast<T>(-1.1L), neg_zero, aNaN, true, FE_INVALID);
TEST_SPECIAL(0.0, neg_zero, aNaN, true, FE_INVALID);
TEST_SPECIAL(neg_zero, neg_zero, aNaN, true, FE_INVALID);
TEST_SPECIAL(min_denormal, neg_zero, aNaN, true, FE_INVALID);
diff --git a/libc/test/src/math/smoke/nan_test.cpp b/libc/test/src/math/smoke/nan_test.cpp
index 46b9e9aa9563ab..da6beb94c7f05d 100644
--- a/libc/test/src/math/smoke/nan_test.cpp
+++ b/libc/test/src/math/smoke/nan_test.cpp
@@ -23,7 +23,7 @@ class LlvmLibcNanTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
auto actual_fp = LIBC_NAMESPACE::fputil::FPBits<double>(result);
auto expected_fp = LIBC_NAMESPACE::fputil::FPBits<double>(bits);
EXPECT_EQ(actual_fp.uintval(), expected_fp.uintval());
- };
+ }
};
TEST_F(LlvmLibcNanTest, NCharSeq) {
diff --git a/libc/test/src/math/smoke/nanf_test.cpp b/libc/test/src/math/smoke/nanf_test.cpp
index dd3124ee9c5112..19d94b40b5ffbd 100644
--- a/libc/test/src/math/smoke/nanf_test.cpp
+++ b/libc/test/src/math/smoke/nanf_test.cpp
@@ -23,7 +23,7 @@ class LlvmLibcNanfTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
auto actual_fp = LIBC_NAMESPACE::fputil::FPBits<float>(result);
auto expected_fp = LIBC_NAMESPACE::fputil::FPBits<float>(bits);
EXPECT_EQ(actual_fp.uintval(), expected_fp.uintval());
- };
+ }
};
TEST_F(LlvmLibcNanfTest, NCharSeq) {
diff --git a/libc/test/src/math/smoke/nanl_test.cpp b/libc/test/src/math/smoke/nanl_test.cpp
index ef3f9c15dafd9f..c7217928e943b0 100644
--- a/libc/test/src/math/smoke/nanl_test.cpp
+++ b/libc/test/src/math/smoke/nanl_test.cpp
@@ -33,7 +33,7 @@ class LlvmLibcNanlTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
auto actual_fp = LIBC_NAMESPACE::fputil::FPBits<long double>(result);
auto expected_fp = LIBC_NAMESPACE::fputil::FPBits<long double>(bits);
EXPECT_EQ(actual_fp.uintval(), expected_fp.uintval());
- };
+ }
};
TEST_F(LlvmLibcNanlTest, NCharSeq) {
diff --git a/libc/test/src/string/memory_utils/op_tests.cpp b/libc/test/src/string/memory_utils/op_tests.cpp
index c6197d1afa266b..dd7619240417ef 100644
--- a/libc/test/src/string/memory_utils/op_tests.cpp
+++ b/libc/test/src/string/memory_utils/op_tests.cpp
@@ -72,7 +72,8 @@ void CopyAdaptor(cpp::span<char> dst, cpp::span<char> src, size_t size) {
FnImpl(as_byte(dst), as_byte(src), size);
}
template <size_t Size, auto FnImpl>
-void CopyBlockAdaptor(cpp::span<char> dst, cpp::span<char> src, size_t size) {
+void CopyBlockAdaptor(cpp::span<char> dst, cpp::span<char> src,
+ size_t /*size*/) {
FnImpl(as_byte(dst), as_byte(src));
}
@@ -153,7 +154,7 @@ void SetAdaptor(cpp::span<char> dst, uint8_t value, size_t size) {
FnImpl(as_byte(dst), value, size);
}
template <size_t Size, auto FnImpl>
-void SetBlockAdaptor(cpp::span<char> dst, uint8_t value, size_t size) {
+void SetBlockAdaptor(cpp::span<char> dst, uint8_t value, size_t /*size*/) {
FnImpl(as_byte(dst), value);
}
@@ -242,7 +243,7 @@ int CmpAdaptor(cpp::span<char> p1, cpp::span<char> p2, size_t size) {
return (int)FnImpl(as_byte(p1), as_byte(p2), size);
}
template <size_t Size, auto FnImpl>
-int CmpBlockAdaptor(cpp::span<char> p1, cpp::span<char> p2, size_t size) {
+int CmpBlockAdaptor(cpp::span<char> p1, cpp::span<char> p2, size_t /*size*/) {
return (int)FnImpl(as_byte(p1), as_byte(p2));
}
diff --git a/libc/test/src/strings/bzero_test.cpp b/libc/test/src/strings/bzero_test.cpp
index 4d4112f4be8ee1..e3091556da2762 100644
--- a/libc/test/src/strings/bzero_test.cpp
+++ b/libc/test/src/strings/bzero_test.cpp
@@ -14,7 +14,7 @@
namespace LIBC_NAMESPACE_DECL {
// Adapt CheckMemset signature to bzero.
-static inline void Adaptor(cpp::span<char> p1, uint8_t value, size_t size) {
+static inline void Adaptor(cpp::span<char> p1, uint8_t /*value*/, size_t size) {
LIBC_NAMESPACE::bzero(p1.begin(), size);
}
diff --git a/libc/utils/MPFRWrapper/MPFRUtils.h b/libc/utils/MPFRWrapper/MPFRUtils.h
index c7a57819f68b79..791936d78cb67d 100644
--- a/libc/utils/MPFRWrapper/MPFRUtils.h
+++ b/libc/utils/MPFRWrapper/MPFRUtils.h
@@ -352,7 +352,7 @@ template <Operation op, typename InputType, typename OutputType>
__attribute__((no_sanitize("address"))) cpp::enable_if_t<
is_valid_operation<op, Inp...
[truncated]
|
|
Running into these MPFR errors: For L413, a For L457, changing |
There was a problem hiding this comment.
Thanks for the patch! Have you tested it with both GCC and clang?
It might be worthwhile to break this up into distinct PRs:
- enable most warnings that dont require any fixes today.
- for each new warning enabled that requires source fixes, submit a distinct PR
It looks like most of the fixes are -Wunused and -Wconversion?
libc/test/UnitTest/LibcTest.h
Outdated
| @@ -81,7 +81,7 @@ struct Message { | |||
| // A trivial object to catch the Message, this enables custom logging and | |||
| // returning from the test function, see LIBC_TEST_SCAFFOLDING_ below. | |||
| struct Failure { | |||
| void operator=(Message msg) {} | |||
| void operator=(Message /*msg*/) {} | |||
There was a problem hiding this comment.
I guess the llvm style guide doesn't say. So I'm ok with what you've chosen.
| count += args.next_var<long double>(); | ||
| count += static_cast<long>(args.next_var<double>()); | ||
| count += static_cast<long>(args.next_var<double>()); | ||
| count += static_cast<long>(args.next_var<long double>()); |
There was a problem hiding this comment.
Mind changing the return type and type of count to just long? int is implied, and isn't specified generally throughout llvm.
Same for check_struct_type below.
| # list(APPEND compile_options "-Wthread-safety") | ||
| # list(APPEND compile_options "-Wglobal-constructors") | ||
| # endif() | ||
| if(NOT CMAKE_COMPILER_IS_GNUCXX) |
There was a problem hiding this comment.
https://cmake.org/cmake/help/latest/variable/CMAKE_COMPILER_IS_GNUCXX.html mentions that this is deprecated and that CMAKE_CXX_COMPILER_ID should be used instead.
It looks fishy to me because "not gcc" isn't necessarily clang, and yet -Wthread-safety is very much so currently a clang specific diagnostic. Instead, the conditional should be phrased as "if clang" rather than "if not gcc."
So if anything this looks like it should be folded into the above else clause that's already testing CMAKE_CXX_COMPILER_ID, or rather a new clause that's `elseif (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
There was a problem hiding this comment.
Made this change, instead of STREQUAL, I used MATCHES so all Clang-like compilers also fall into this case.
I have only used AppleClang for this. I was hoping the CI has GCC and LLVM's Clang as well. To confirm, the first PR would comprise of the discussed CMake fixes && enable the warnings that don't require fixes I'll raise those in the coming days! |
vinay-deshmukh
changed the title
These warnings will be enabled one by one in subsequent PRs.
…into vinay-issue-119281
vinay-deshmukh
commented
Jan 16, 2025
| list(APPEND compile_options "-Wno-implicit-float-conversion") | ||
| list(APPEND compile_options "-Wno-extra-semi") | ||
|
|
||
|
|
||
| list(APPEND compile_options "-Wno-c99-extensions") | ||
| list(APPEND compile_options "-Wno-gnu-imaginary-constant") |
There was a problem hiding this comment.
Should we also remove these no-* warnings? I didn't notice them earlier, but while I do the other PRs, I can take a look at enabling these as well?
There was a problem hiding this comment.
Not in this PR. I'm not familiar with these two diagnostics in particular, so I'm not certain why they were explicitly disabled.
vinay-deshmukh
commented
Jan 16, 2025
| list(APPEND compile_options "-Wno-c99-extensions") | ||
| list(APPEND compile_options "-Wno-gnu-imaginary-constant") | ||
|
|
||
| endif() |
There was a problem hiding this comment.
There is no else() block now, Should we do a message(WARN ...) to say, "Most warnings are enabled on *Clang$. Prefer using that compiler ..." ?
There was a problem hiding this comment.
It's fine; today we only support gcc and clang. If someone cares to support a third compiler for llvm-libc, we'd like to know about it.
Warning text for `-Wmissing-field-initializers` |
| # Silence this warning because _Complex is a part of C99. | ||
| if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") | ||
| if(NOT c_test) | ||
| list(APPEND compile_options "-fext-numeric-literals") | ||
| endif() | ||
| else() | ||
| elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang$") |
There was a problem hiding this comment.
How come you use MATCHES with $ rather than STREQUAL without $?
There was a problem hiding this comment.
So, since I'm using AppleClang on a macOS m1, I figured I could use MATCHES to match Clang and other clang-like compilers
| list(APPEND compile_options "-Wno-shorten-64-to-32") | ||
| list(APPEND compile_options "-Wno-float-conversion") | ||
| list(APPEND compile_options "-Wno-implicit-float-conversion") | ||
| list(APPEND compile_options "-Wno-extra-semi") |
There was a problem hiding this comment.
I think it's better to just start submitting these fixes first, then revisit this PR. Here's an example for -Wextra-semi:
#123783
There was a problem hiding this comment.
Oh ok, I can do that, let me raise the ones for the other issues
There was a problem hiding this comment.
The "simple" warning fixes that could be done are in: #124036.
All the other warnings seem to need fixes in multiple files and aren't straight forward to enable/trigger (i.e. to solve unused parameter, I need to enable Wextra which ends up raising a few other issues as well, like "omitting parameter name is a C23 extension").
In that case, IMO it would be a good idea to merge this PR after #124036, and then in each subsequent PR, work on removing the -Wno-* (that are disabled in this PR) one by one in subsequent PRs. This way, each subsequent PR is clearly delineated in terms of "what" to fix.
There was a problem hiding this comment.
Sure. I don't mind if you need to enable -Wextra and add a few more -Wno-* options in the same PR, then work towards eliminating those new -Wno-* flags.
nickdesaulniers
pushed a commit
that referenced
this pull request
Feb 7, 2025
…warnings (#124036) * Enabled `-Wimplicit-fallthrough` * Enabled `-Wwrite-strings` * Enabled non-GCC warnings * Fix non-GCC to mean `clang` * Move `-Wstrict-prototypes` to common section See: #122835 (comment) Relates to #119281
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Feb 7, 2025
…nd non-GCC warnings (#124036) * Enabled `-Wimplicit-fallthrough` * Enabled `-Wwrite-strings` * Enabled non-GCC warnings * Fix non-GCC to mean `clang` * Move `-Wstrict-prototypes` to common section See: llvm/llvm-project#122835 (comment) Relates to #119281
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…warnings (llvm#124036) * Enabled `-Wimplicit-fallthrough` * Enabled `-Wwrite-strings` * Enabled non-GCC warnings * Fix non-GCC to mean `clang` * Move `-Wstrict-prototypes` to common section See: llvm#122835 (comment) Relates to llvm#119281
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
Old PR description
* `-Wno-global-constructors` is triggered by `TEST_F` implementation. To enable this warning, we will need a bigger refactor of how the `TEST_F` macro is implemented. If needed, I can tackle that in this PR * Specifically, this code triggers that warning since the ctor runs in the global/static scope. https://github.com/llvm/llvm-project/blob/2201164477982c2bd20fa2e925f567585c390805/libc/test/UnitTest/LibcTest.h#L400 * Other warnings were relatively straightforward to fix.