From 3a7e06ca48f4b42bbacb70ea7f59bf09d6aa0b13 Mon Sep 17 00:00:00 2001 From: Alex Ainscow Date: Wed, 13 Aug 2025 12:03:21 +0100 Subject: [PATCH] src: Add sign-compare warnings to clang For a while, GCC has generated warnings about sign errors. A common mistake if compiling with clang was to accidentally introduce signedness errors, which were picked up by the GCC builds. This occurs due to an inconsistency in -Wall implementation between clang and gcc: gcc includes sign-compare, clang does not. See: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wall vs https://clang.llvm.org/docs/DiagnosticsReference.html#wall Note that sign-compare is included under -Wextra for clang: https://clang.llvm.org/docs/DiagnosticsReference.html#wextra Clang will now generate similar warnings with -Wsign-compare: https://clang.llvm.org/docs/DiagnosticsReference.html#wsign-compare Interestingly, if specified on its own, -Wsign-compare will include C, whereas gcc -Wall affects C++ only. Therefore we must work around this in the make file to emulate the GCC behaviour in clang builds. Also fix a couple of warnings found in some tests. Signed-off-by: Alex Ainscow --- src/CMakeLists.txt | 4 ++++ src/test/osd/TestECBackend.cc | 5 ++--- src/test/osd/test_scrubber_be.cc | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ada75e281c5..a851c25312c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -113,6 +113,10 @@ if(NOT MSVC) add_compile_options($<$:-Wno-ignored-qualifiers>) endif() +if (CMAKE_CXX_COMPILER_ID STREQUAL Clang) + add_compile_options($<$:-Wsign-compare>) +endif() + add_compile_options($<$:-ftemplate-depth-1024>) # Because Boost can't be bothered to not include its own deprecated headers diff --git a/src/test/osd/TestECBackend.cc b/src/test/osd/TestECBackend.cc index ca299abd963..664969be295 100644 --- a/src/test/osd/TestECBackend.cc +++ b/src/test/osd/TestECBackend.cc @@ -158,12 +158,12 @@ public: // for each missing shard. for (auto a : available) { minimum_set.insert(a); - if (minimum_set.size() == data_chunk_count) { + if (std::cmp_equal(minimum_set.size(), data_chunk_count)) { break; } } - if (minimum_set.size() != data_chunk_count) { + if (std::cmp_not_equal(minimum_set.size(), data_chunk_count)) { minimum_set.clear(); return -EIO; // Cannot recover. } @@ -174,7 +174,6 @@ public: } return 0; } - [[deprecated]] int minimum_to_decode(const std::set &want_to_read, const std::set &available, diff --git a/src/test/osd/test_scrubber_be.cc b/src/test/osd/test_scrubber_be.cc index bc6db59ac17..60e995aed5f 100644 --- a/src/test/osd/test_scrubber_be.cc +++ b/src/test/osd/test_scrubber_be.cc @@ -118,7 +118,7 @@ class TestPg : public PgScrubBeListener { } for (shard_id_t i; i < get_ec_sinfo().get_k(); ++i) { - for (int j = 0; j < get_ec_sinfo().get_chunk_size(); j++) { + for (int j = 0; std::cmp_less(j, get_ec_sinfo().get_chunk_size()); j++) { encode_map.at(i).c_str()[j] = chunks[j + (get_ec_sinfo().get_chunk_size() * i.id)]; for (shard_id_t k{static_cast(get_ec_sinfo().get_k_plus_m())}; -- 2.47.3