Fix warnings in Wall and Wextra for clang and gcc

Author: cschreib

.github/workflows/cmake.yml

@@ -13,12 +13,12 @@ jobs:
  fail-fast: false
  matrix:
  platform:
- - { name: Ubuntu GCC, os: ubuntu-latest, compiler: g++, arch: "64", cmakepp: "", flags: "-DCMAKE_CXX_COMPILER=g++ -DCMAKE_CXX_FLAGS=\"--coverage -Wall\""}
- - { name: Ubuntu Clang, os: ubuntu-latest, compiler: clang++, arch: "64", cmakepp: "", flags: "-DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CXX_FLAGS=\"-stdlib=libc++ -Weverything\""}
- - { name: Windows 32, os: windows-latest, compiler: vs2019, arch: "32", cmakepp: "", flags: "-A Win32 -DCMAKE_CXX_FLAGS=\"/W4\""}
- - { name: Windows 64, os: windows-latest, compiler: vs2019, arch: "64", cmakepp: "", flags: "-A x64 -DCMAKE_CXX_FLAGS=\"/W4\""}
- - { name: MacOS, os: macos-latest, compiler: clang++, arch: "64", cmakepp: "", flags: "-DCMAKE_CXX_FLAGS=\"-Weverything\""}
- - { name: WebAssembly, os: ubuntu-latest, compiler: em++, arch: "32", cmakepp: "emcmake", flags: "-DCMAKE_CXX_FLAGS=\"-Weverything -s DISABLE_EXCEPTION_CATCHING=0\" -DCMAKE_CROSSCOMPILING_EMULATOR=node"}
+ - { name: Ubuntu GCC, os: ubuntu-latest, compiler: g++, arch: "64", cmakepp: "", flags: "-DCMAKE_CXX_COMPILER=g++ -DCMAKE_CXX_FLAGS=\"--coverage -Wall -Wextra -Werror\""}
+ - { name: Ubuntu Clang, os: ubuntu-latest, compiler: clang++, arch: "64", cmakepp: "", flags: "-DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CXX_FLAGS=\"-stdlib=libc++ -Wall -Wextra -Werror\""}
+ - { name: Windows 32, os: windows-latest, compiler: vs2019, arch: "32", cmakepp: "", flags: "-A Win32 -DCMAKE_CXX_FLAGS=\"/W4 /Wx\""}
+ - { name: Windows 64, os: windows-latest, compiler: vs2019, arch: "64", cmakepp: "", flags: "-A x64 -DCMAKE_CXX_FLAGS=\"/W4 /Wx\""}
+ - { name: MacOS, os: macos-latest, compiler: clang++, arch: "64", cmakepp: "", flags: "-DCMAKE_CXX_FLAGS=\"-Wall -Wextra -Werror\""}
+ - { name: WebAssembly, os: ubuntu-latest, compiler: em++, arch: "32", cmakepp: "emcmake", flags: "-DCMAKE_CXX_FLAGS=\"-Wall -Wextra -Werror -s DISABLE_EXCEPTION_CATCHING=0\" -DCMAKE_CROSSCOMPILING_EMULATOR=node"}
  build-type:
  - Release
  - Debug

tests/runtime_tests_observer_assignment_copy.cpp

@@ -219,7 +219,10 @@ TEMPLATE_LIST_TEST_CASE(
  {
  TestType ptr = make_pointer_deleter_1<TestType>();
  observer_ptr<TestType> optr{ptr};
+ WARNING_PUSH;
+ WARNING_DISABLE_SELF_ASSIGN;
  optr = optr;
+ WARNING_POP;
 
  CHECK(optr.get() == ptr.get());
  CHECK(optr.expired() == false);
@@ -237,7 +240,10 @@ TEMPLATE_LIST_TEST_CASE(
 
  {
  observer_ptr<TestType> optr;
+ WARNING_PUSH;
+ WARNING_DISABLE_SELF_ASSIGN;
  optr = optr;
+ WARNING_POP;
 
  CHECK(optr.get() == nullptr);
  CHECK(optr.expired() == true);

tests/runtime_tests_observer_cast_copy.cpp

@@ -147,10 +147,9 @@ TEMPLATE_LIST_TEST_CASE("observer dynamic_cast copy from empty", "[cast],[observ
  {
  auto run_test =
  [&]<typename start_type, typename cast_type, typename expected_return_type>() {
- TestType ptr0 = make_empty_pointer_deleter_1<TestType>();
- get_object<TestType>* raw_ptr = ptr0.get();
- observer_ptr<start_type> optr1 = ptr0;
- auto optr2 = oup::dynamic_pointer_cast<cast_type>(optr1);
+ TestType ptr0 = make_empty_pointer_deleter_1<TestType>();
+ observer_ptr<start_type> optr1 = ptr0;
+ auto optr2 = oup::dynamic_pointer_cast<cast_type>(optr1);
 
  using return_type = std::remove_cv_t<decltype(optr2)>;
 

tests/runtime_tests_observer_cast_move.cpp

@@ -147,9 +147,8 @@ TEMPLATE_LIST_TEST_CASE("observer dynamic_cast move from empty", "[cast],[observ
  {
  auto run_test =
  [&]<typename start_type, typename cast_type, typename expected_return_type>() {
- TestType ptr0 = make_empty_pointer_deleter_1<TestType>();
- get_object<TestType>* raw_ptr = ptr0.get();
- observer_ptr<start_type> optr1 = ptr0;
+ TestType ptr0 = make_empty_pointer_deleter_1<TestType>();
+ observer_ptr<start_type> optr1 = ptr0;
  auto optr2 = oup::dynamic_pointer_cast<cast_type>(std::move(optr1));
 
  using return_type = std::remove_cv_t<decltype(optr2)>;
@@ -180,9 +179,8 @@ TEMPLATE_LIST_TEST_CASE(
  memory_tracker mem_track;
 
  {
- TestType ptr0 = make_pointer_deleter_1<TestType>();
- get_object<TestType>* raw_ptr = ptr0.get();
- base_observer_ptr<TestType> optr1 = ptr0;
+ TestType ptr0 = make_pointer_deleter_1<TestType>();
+ base_observer_ptr<TestType> optr1 = ptr0;
  auto optr2 = oup::dynamic_pointer_cast<test_object_dead_end>(std::move(optr1));
 
  using return_type = std::remove_cv_t<decltype(optr2)>;

tests/runtime_tests_owner_cast_move.cpp

@@ -158,10 +158,9 @@ TEMPLATE_LIST_TEST_CASE("owner dynamic_cast move from empty", "[cast],[owner]",
  {
  auto run_test =
  [&]<typename start_type, typename cast_type, typename expected_return_type>() {
- TestType ptr0 = make_empty_pointer_deleter_1<TestType>();
- get_object<TestType>* raw_ptr = ptr0.get();
- start_type ptr1 = std::move(ptr0);
- auto ptr2 = oup::dynamic_pointer_cast<cast_type>(std::move(ptr1));
+ TestType ptr0 = make_empty_pointer_deleter_1<TestType>();
+ start_type ptr1 = std::move(ptr0);
+ auto ptr2 = oup::dynamic_pointer_cast<cast_type>(std::move(ptr1));
 
  using return_type = std::remove_cv_t<decltype(ptr2)>;
 

tests/snatch.cpp

@@ -11,17 +11,17 @@
 # define SNATCH_WITH_CXXOPTS 0
 #endif
 
-namespace snatch::impl::color {
-const char* error_start = "\x1b[1;31m";
-const char* warning_start = "\x1b[1;33m";
-const char* status_start = "\x1b[1;36m";
-const char* fail_start = "\x1b[1;31m";
-const char* skipped_start = "\x1b[1;33m";
-const char* pass_start = "\x1b[1;32m";
-const char* highlight1_start = "\x1b[1;35m";
-const char* highlight2_start = "\x1b[1;36m";
-const char* reset = "\x1b[0m";
-} // namespace snatch::impl::color
+namespace { namespace color {
+const char* error_start [[maybe_unused]]  = "\x1b[1;31m";
+const char* warning_start [[maybe_unused]]  = "\x1b[1;33m";
+const char* status_start [[maybe_unused]]  = "\x1b[1;36m";
+const char* fail_start [[maybe_unused]]  = "\x1b[1;31m";
+const char* skipped_start [[maybe_unused]]  = "\x1b[1;33m";
+const char* pass_start [[maybe_unused]]  = "\x1b[1;32m";
+const char* highlight1_start [[maybe_unused]] = "\x1b[1;35m";
+const char* highlight2_start [[maybe_unused]] = "\x1b[1;36m";
+const char* reset [[maybe_unused]]  = "\x1b[0m";
+}} // namespace ::color
 
 namespace {
 using namespace snatch;
@@ -133,9 +133,13 @@ constexpr const char* get_format_code() noexcept {
 
 template<typename T>
 bool append_fmt(basic_small_string& ss, T value) noexcept {
- const std::size_t length = std::snprintf(ss.end(), 0, get_format_code<T>(), value);
+ const int return_code = std::snprintf(ss.end(), 0, get_format_code<T>(), value);
+ if (return_code < 0) {
+ return false;
+ }
 
- const bool could_fit = length <= ss.available();
+ const std::size_t length = static_cast<std::size_t>(return_code);
+ const bool could_fit = length <= ss.available();
 
  const std::size_t offset = ss.size();
  const std::size_t prev_space = ss.available();
@@ -205,19 +209,21 @@ void truncate_end(basic_small_string& ss) noexcept {
 } // namespace snatch::impl
 
 namespace snatch::matchers {
-contains_substring::contains_substring(std::string_view pattern) noexcept : pattern(pattern) {}
+contains_substring::contains_substring(std::string_view pattern) noexcept :
+ substring_pattern(pattern) {}
 
 bool contains_substring::match(std::string_view message) const noexcept {
- return message.find(pattern) != message.npos;
+ return message.find(substring_pattern) != message.npos;
 }
 
 std::string_view contains_substring::describe_fail(std::string_view message) const noexcept {
- description.clear();
- if (!append(description, "could not find '", pattern, "' in '", message, "'")) {
- truncate_end(description);
+ description_buffer.clear();
+ if (!append(
+ description_buffer, "could not find '", substring_pattern, "' in '", message, "'")) {
+ truncate_end(description_buffer);
  }
 
- return description.str();
+ return description_buffer.str();
 }
 
 with_what_contains::with_what_contains(std::string_view pattern) noexcept :

tests/snatch/snatch.hpp

@@ -531,8 +531,8 @@ const char* proxy<std::tuple<Args...>>::operator=(const F& func) noexcept {
 
 namespace snatch::matchers {
 struct contains_substring {
- mutable impl::small_string<max_matcher_msg_length> description;
- std::string_view pattern;
+ mutable impl::small_string<max_matcher_msg_length> description_buffer;
+ std::string_view substring_pattern;
 
  explicit contains_substring(std::string_view pattern) noexcept;
 
@@ -563,6 +563,26 @@ struct with_what_contains : private contains_substring {
 #define TESTING_MACRO_CONCAT(x, y) TESTING_CONCAT_IMPL(x, y)
 #define TESTING_EXPR(x) snatch::impl::expression{} <= x
 
+// clang-format off
+#if defined(__clang__)
+# define WARNING_PUSH _Pragma("clang diagnostic push")
+# define WARNING_POP _Pragma("clang diagnostic pop")
+# define WARNING_DISABLE_PARENTHESES do {} while (0)
+#elif defined(__GNUC__)
+# define WARNING_PUSH _Pragma("GCC diagnostic push")
+# define WARNING_POP _Pragma("GCC diagnostic pop")
+# define WARNING_DISABLE_PARENTHESES _Pragma("GCC diagnostic ignored \"-Wparentheses\"")
+#elif defined(_MSC_VER)
+# define WARNING_PUSH do {} while (0)
+# define WARNING_POP do {} while (0)
+# define WARNING_DISABLE_PARENTHESES do {} while (0)
+#else
+# define WARNING_PUSH do {} while (0)
+# define WARNING_POP do {} while (0)
+# define WARNING_DISABLE_PARENTHESES do {} while (0)
+#endif
+// clang-format on
+
 #define TEST_CASE(NAME, TAGS) \
  static const char* TESTING_MACRO_CONCAT(test_id_, __COUNTER__) = \
  snatch::tests.add(NAME, TAGS) = \
@@ -577,7 +597,10 @@ struct with_what_contains : private contains_substring {
  do { \
  ++CURRENT_CASE.tests; \
  if (!(EXP)) { \
+ WARNING_PUSH; \
+ WARNING_DISABLE_PARENTHESES; \
  const auto EXP2 = TESTING_EXPR(EXP); \
+ WARNING_POP; \
  snatch::tests.print_failure(); \
  snatch::tests.print_location(CURRENT_CASE, __FILE__, __LINE__); \
  snatch::tests.print_details_expr("REQUIRE", #EXP, EXP2); \
@@ -589,7 +612,10 @@ struct with_what_contains : private contains_substring {
  do { \
  ++CURRENT_CASE.tests; \
  if (!(EXP)) { \
+ WARNING_PUSH; \
+ WARNING_DISABLE_PARENTHESES; \
  const auto EXP2 = TESTING_EXPR(EXP); \
+ WARNING_POP; \
  snatch::tests.print_failure(); \
  snatch::tests.print_location(CURRENT_CASE, __FILE__, __LINE__); \
  snatch::tests.print_details_expr("CHECK", #EXP, EXP2); \

tests/speed_benchmark.cpp

@@ -86,7 +86,7 @@ void do_report(const char* name, const R& which) {
 }
 
 double median(std::vector<double> v) {
- const auto n = v.size() / 2;
+ const auto n = static_cast<std::ptrdiff_t>(v.size() / 2);
  std::nth_element(v.begin(), v.begin() + n, v.end());
  return *(v.begin() + n);
 }
@@ -172,7 +172,7 @@ int main() {
  std::cout << std::endl;
 
  std::cout << "|---|---|";
- for (const auto& t : cols) {
+ for (const auto& t [[maybe_unused]] : cols) {
  std::cout << "---|";
  }
  std::cout << std::endl;

tests/testing.hpp

@@ -1,19 +1 @@
-// #include <catch2/catch_template_test_macros.hpp>
-
-// // Replace Catch2's REQUIRE_THROWS_AS, which allocates on Windows;
-// // this confuses our memory leak checks.
-// #undef REQUIRE_THROWS_AS
-// #define REQUIRE_THROWS_AS(EXPRESSION, EXCEPTION) \
-// do { \
-// try { \
-// EXPRESSION; \
-// FAIL("no exception thrown"); \
-// } catch (const EXCEPTION&) { \
-// } catch (const std::exception& e) { \
-// FAIL(e.what()); \
-// } catch (...) { \
-// FAIL("unexpected exception thrown"); \
-// } \
-// } while (0)
-
 #include "snatch/snatch.hpp"

tests/tests_common.cpp

@@ -16,14 +16,14 @@ test_object::test_object() {
  next_test_object_constructor_throws = false;
 
  if constexpr (debug_instances) {
- std::printf("%p creation throws\n", this);
+ std::printf("%p creation throws\n", static_cast<void*>(this));
  }
 
  throw throw_constructor{};
  }
 
  if constexpr (debug_instances) {
- std::printf("%p created\n", this);
+ std::printf("%p created\n", static_cast<void*>(this));
  }
 
  ++instances;
@@ -34,22 +34,22 @@ test_object::test_object(state s) : state_(s) {
  next_test_object_constructor_throws = false;
 
  if constexpr (debug_instances) {
- std::printf("%p creation throws\n", this);
+ std::printf("%p creation throws\n", static_cast<void*>(this));
  }
 
  throw throw_constructor{};
  }
 
  if constexpr (debug_instances) {
- std::printf("%p created\n", this);
+ std::printf("%p created\n", static_cast<void*>(this));
  }
 
  ++instances;
 }
 
 test_object::~test_object() noexcept {
  if constexpr (debug_instances) {
- std::printf("%p deleted\n", this);
+ std::printf("%p deleted\n", static_cast<void*>(this));
  }
 
  --instances;