From 672cc0032eb28d69fbdd22c9463253c89d7a6f30 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Tue, 13 Aug 2024 15:10:15 -0700 Subject: [PATCH] Fix `-Wuseless-cast` compiler warnings (#1145) **Issue:** https://github.com/awslabs/aws-c-common/issues/973 This warning exists in GCC only (not Clang), and for C++ only (not C) **Description of changes:** - Add `-Wuseless-cast` warning to the "header checker" - Remove useless casts in `array_list.inl` - Ignore `-Wuseless-cast` warning in `math.inl` - The tricky part is that SOME platforms (Apple and BSD variants) define `size_t` as `unsigned long` and require casting to `uint64_t` (aka `unsigned long long`). While other platforms define `size_t` as `unsigned long long` and warn about useless casts if you cast it to `uint64_t`. My first commit attempted to only do casts on those platforms, but it was more complex than simply ignoring the warning. --- cmake/AwsCheckHeaders.cmake | 23 +++++++++++++++++++---- include/aws/common/array_list.inl | 2 +- include/aws/common/math.inl | 9 ++++++++- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/cmake/AwsCheckHeaders.cmake b/cmake/AwsCheckHeaders.cmake index 2e62e0ad5..562a3739c 100644 --- a/cmake/AwsCheckHeaders.cmake +++ b/cmake/AwsCheckHeaders.cmake @@ -63,13 +63,22 @@ function(aws_check_headers_internal target std is_cxx) C_STANDARD 99 ) - # Ensure our headers can be included by an application with its warnings set very high + # Ensure our headers can be included by an application with its warnings set very high. + # Most compiler options are universal, but some are C++ only. + set(compiler_options_all "") + set(compiler_options_cxx_only "") if(MSVC) - # MSVC complains about windows' own header files. Use /W4 instead of /Wall - target_compile_options(${HEADER_CHECKER_LIB} PRIVATE /W4 /WX) + # MSVC complains about windows' own header files. Use /W4 instead of /Wall + list(APPEND compiler_options_all /W4 /WX) else() - target_compile_options(${HEADER_CHECKER_LIB} PRIVATE -Wall -Wextra -Wpedantic -Werror) + list(APPEND compiler_options_all -Wall -Wextra -Wpedantic -Werror) + + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # -Wuseless-cast requested by https://github.com/awslabs/aws-c-common/issues/973 + list(APPEND compiler_options_cxx_only -Wuseless-cast) + endif() endif() + target_compile_options(${HEADER_CHECKER_LIB} PRIVATE ${compiler_options_all}) foreach(header IN LISTS ARGN) if (NOT ${header} MATCHES "\\.inl$") @@ -78,6 +87,7 @@ function(aws_check_headers_internal target std is_cxx) file(RELATIVE_PATH include_path "${CMAKE_CURRENT_SOURCE_DIR}/include" ${header}) # replace non-alphanumeric characters with underscores string(REGEX REPLACE "[^a-zA-Z0-9]" "_" unique_token ${include_path}) + # test compiling header from a .cpp and .c file (or just .cpp if this header IS_CXX) set(c_file "${HEADER_CHECKER_ROOT}/headerchecker_${unique_token}.c") set(cpp_file "${HEADER_CHECKER_ROOT}/headerchecker_${unique_token}.cpp") # include header twice to check for include-guards @@ -88,6 +98,11 @@ function(aws_check_headers_internal target std is_cxx) file(WRITE "${c_file}" "#include <${include_path}>\n#include <${include_path}>\nint ${unique_token}_c;\n") target_sources(${HEADER_CHECKER_LIB} PUBLIC "${c_file}") endif() + + # for .cpp file, apply C++ only compiler options + if(compiler_options_cxx_only) + set_source_files_properties(${cpp_file} PROPERTIES COMPILE_OPTIONS ${compiler_options_cxx_only}) + endif() endif() endforeach(header) endfunction() diff --git a/include/aws/common/array_list.inl b/include/aws/common/array_list.inl index 42f447e92..5472104c6 100644 --- a/include/aws/common/array_list.inl +++ b/include/aws/common/array_list.inl @@ -123,7 +123,7 @@ AWS_STATIC_IMPL void aws_array_list_clean_up_secure(struct aws_array_list *AWS_RESTRICT list) { AWS_PRECONDITION(AWS_IS_ZEROED(*list) || aws_array_list_is_valid(list)); if (list->alloc && list->data) { - aws_secure_zero((void *)list->data, list->current_size); + aws_secure_zero(list->data, list->current_size); aws_mem_release(list->alloc, list->data); } diff --git a/include/aws/common/math.inl b/include/aws/common/math.inl index 2081fbcca..ad3590623 100644 --- a/include/aws/common/math.inl +++ b/include/aws/common/math.inl @@ -51,7 +51,12 @@ AWS_EXTERN_C_BEGIN #ifdef _MSC_VER # pragma warning(push) # pragma warning(disable : 4127) /*Disable "conditional expression is constant" */ -#endif /* _MSC_VER */ +#elif defined(__GNUC__) +# pragma GCC diagnostic push +# if defined(__cplusplus) && !defined(__clang__) +# pragma GCC diagnostic ignored "-Wuseless-cast" /* Warning is C++ only (not C), and GCC only (not clang) */ +# endif +#endif AWS_STATIC_IMPL uint64_t aws_sub_u64_saturating(uint64_t a, uint64_t b) { return a <= b ? 0 : a - b; @@ -190,6 +195,8 @@ AWS_STATIC_IMPL int aws_round_up_to_power_of_two(size_t n, size_t *result) { #ifdef _MSC_VER # pragma warning(pop) +#elif defined(__GNUC__) +# pragma GCC diagnostic pop #endif /* _MSC_VER */ AWS_STATIC_IMPL uint8_t aws_min_u8(uint8_t a, uint8_t b) {