From 34da61a62c6fced32caa28f0129be9a47d87f5e1 Mon Sep 17 00:00:00 2001 From: leftibot Date: Mon, 13 Apr 2026 23:18:26 -0600 Subject: [PATCH 1/2] Fix #76: Remove redundant -Wsign-conversion from CLANG_WARNINGS -Wconversion already implies -Wsign-conversion for both GCC and Clang, making the explicit -Wsign-conversion flag redundant in the CLANG_WARNINGS list. Since GCC_WARNINGS inherits from CLANG_WARNINGS, this removal applies to both compilers. Added a regression test to prevent reintroduction of the redundancy. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmake/CompilerWarnings.cmake | 1 - test/CMakeLists.txt | 5 +++ test/cmake/test_no_redundant_warnings.cmake | 34 +++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/cmake/test_no_redundant_warnings.cmake diff --git a/cmake/CompilerWarnings.cmake b/cmake/CompilerWarnings.cmake index 281ea1b2..ebfc6baa 100644 --- a/cmake/CompilerWarnings.cmake +++ b/cmake/CompilerWarnings.cmake @@ -51,7 +51,6 @@ function( -Woverloaded-virtual # warn if you overload (not override) a virtual function -Wpedantic # warn if non-standard C++ is used -Wconversion # warn on type conversions that may lose data - -Wsign-conversion # warn on sign conversions -Wnull-dereference # warn if a null dereference is detected -Wdouble-promotion # warn if float is implicit promoted to double -Wformat=2 # warn on security issues around functions that format output (ie printf) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8679286f..703a663d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -88,6 +88,11 @@ catch_discover_tests( OUTPUT_SUFFIX .xml) +# Verify no redundant warning flags in CompilerWarnings.cmake +add_test( + NAME cmake.no_redundant_warnings + COMMAND ${CMAKE_COMMAND} -DPROJECT_ROOT=${CMAKE_SOURCE_DIR} -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/test_no_redundant_warnings.cmake) + # Disable the constexpr portion of the test, and build again this allows us to have an executable that we can debug when # things go wrong with the constexpr testing add_executable(relaxed_constexpr_tests constexpr_tests.cpp) diff --git a/test/cmake/test_no_redundant_warnings.cmake b/test/cmake/test_no_redundant_warnings.cmake new file mode 100644 index 00000000..dd48489a --- /dev/null +++ b/test/cmake/test_no_redundant_warnings.cmake @@ -0,0 +1,34 @@ +# Test that CLANG_WARNINGS does not contain -Wsign-conversion, +# since -Wconversion already implies it for both GCC and Clang. + +file(STRINGS "${PROJECT_ROOT}/cmake/CompilerWarnings.cmake" lines) + +set(in_clang_block FALSE) +set(found_sign_conversion FALSE) + +foreach(line IN LISTS lines) + if(line MATCHES "set\\(CLANG_WARNINGS") + set(in_clang_block TRUE) + endif() + + if(in_clang_block AND line MATCHES "-Wsign-conversion") + set(found_sign_conversion TRUE) + endif() + + # The closing paren of the set() ends the block + if(in_clang_block AND line MATCHES "^[ \t]*\\)[ \t]*$") + break() + endif() +endforeach() + +if(NOT in_clang_block) + message(FATAL_ERROR "Could not find CLANG_WARNINGS block in CompilerWarnings.cmake") +endif() + +if(found_sign_conversion) + message(FATAL_ERROR + "-Wsign-conversion is redundant in CLANG_WARNINGS: " + "-Wconversion already implies -Wsign-conversion for both GCC and Clang") +endif() + +message(STATUS "PASS: No redundant -Wsign-conversion in CLANG_WARNINGS") From 2750021f1c88a0472cbfa926c462ce7c776bf9d4 Mon Sep 17 00:00:00 2001 From: leftibot Date: Wed, 15 Apr 2026 14:19:56 -0600 Subject: [PATCH 2/2] Address review: remove redundant warning regression test Requested by @lefticus in PR #149 review. Co-Authored-By: Claude Opus 4.6 (1M context) --- test/CMakeLists.txt | 5 --- test/cmake/test_no_redundant_warnings.cmake | 34 --------------------- 2 files changed, 39 deletions(-) delete mode 100644 test/cmake/test_no_redundant_warnings.cmake diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 703a663d..8679286f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -88,11 +88,6 @@ catch_discover_tests( OUTPUT_SUFFIX .xml) -# Verify no redundant warning flags in CompilerWarnings.cmake -add_test( - NAME cmake.no_redundant_warnings - COMMAND ${CMAKE_COMMAND} -DPROJECT_ROOT=${CMAKE_SOURCE_DIR} -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/test_no_redundant_warnings.cmake) - # Disable the constexpr portion of the test, and build again this allows us to have an executable that we can debug when # things go wrong with the constexpr testing add_executable(relaxed_constexpr_tests constexpr_tests.cpp) diff --git a/test/cmake/test_no_redundant_warnings.cmake b/test/cmake/test_no_redundant_warnings.cmake deleted file mode 100644 index dd48489a..00000000 --- a/test/cmake/test_no_redundant_warnings.cmake +++ /dev/null @@ -1,34 +0,0 @@ -# Test that CLANG_WARNINGS does not contain -Wsign-conversion, -# since -Wconversion already implies it for both GCC and Clang. - -file(STRINGS "${PROJECT_ROOT}/cmake/CompilerWarnings.cmake" lines) - -set(in_clang_block FALSE) -set(found_sign_conversion FALSE) - -foreach(line IN LISTS lines) - if(line MATCHES "set\\(CLANG_WARNINGS") - set(in_clang_block TRUE) - endif() - - if(in_clang_block AND line MATCHES "-Wsign-conversion") - set(found_sign_conversion TRUE) - endif() - - # The closing paren of the set() ends the block - if(in_clang_block AND line MATCHES "^[ \t]*\\)[ \t]*$") - break() - endif() -endforeach() - -if(NOT in_clang_block) - message(FATAL_ERROR "Could not find CLANG_WARNINGS block in CompilerWarnings.cmake") -endif() - -if(found_sign_conversion) - message(FATAL_ERROR - "-Wsign-conversion is redundant in CLANG_WARNINGS: " - "-Wconversion already implies -Wsign-conversion for both GCC and Clang") -endif() - -message(STATUS "PASS: No redundant -Wsign-conversion in CLANG_WARNINGS")