From 9d0754ba6b6de882d55bdfb9439a84c9aa85dd3e Mon Sep 17 00:00:00 2001 From: "Klein, Thorsten" Date: Thu, 23 Oct 2025 17:38:53 +0200 Subject: [PATCH] Add common Clang warnings to cxx_base_flags and fix existing issues This commit fixes several Clang warnings and updates cxx_base_flags to always enable them, ensuring consistent warning coverage across builds. Enabled additional warnings: -Wswitch-default -Wswitch-enum -Wextra -Wcast-align -Wunused -Wunreachable-code --- googlemock/include/gmock/gmock-matchers.h | 4 ++-- googlemock/src/gmock-matchers.cc | 4 ++++ googlemock/src/gmock-spec-builders.cc | 1 + googlemock/test/gmock-function-mocker_test.cc | 2 +- googlemock/test/gmock-matchers-containers_test.cc | 2 ++ googletest/cmake/internal_utils.cmake | 12 +++++++++++- .../gtest/internal/gtest-death-test-internal.h | 2 ++ googletest/src/gtest.cc | 9 +++++++++ googletest/test/googletest-death-test-test.cc | 4 ++-- googletest/test/googletest-port-test.cc | 2 +- googletest/test/gtest_environment_test.cc | 1 + googletest/test/gtest_unittest.cc | 4 ++-- 12 files changed, 38 insertions(+), 9 deletions(-) diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index 236d3e5f0b..13d92cf604 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -5777,7 +5777,7 @@ PolymorphicMatcher> ThrowsMessage( } \ template \ bool name##Matcher::gmock_Impl::MatchAndExplain( \ - const arg_type& arg, \ + [[maybe_unused]] const arg_type& arg, \ [[maybe_unused]] ::testing::MatchResultListener* result_listener) const #define MATCHER_P(name, p0, description) \ @@ -5862,7 +5862,7 @@ PolymorphicMatcher> ThrowsMessage( template \ bool full_name:: \ gmock_Impl::MatchAndExplain( \ - const arg_type& arg, \ + [[maybe_unused]] const arg_type& arg, \ [[maybe_unused]] ::testing::MatchResultListener* result_listener) \ const diff --git a/googlemock/src/gmock-matchers.cc b/googlemock/src/gmock-matchers.cc index 277add6b62..a223000ff9 100644 --- a/googlemock/src/gmock-matchers.cc +++ b/googlemock/src/gmock-matchers.cc @@ -301,6 +301,8 @@ void UnorderedElementsAreMatcherImplBase::DescribeToImpl( case UnorderedMatcherRequire::Subset: *os << "an injection from elements to requirements exists such that:\n"; break; + default: + break; } const char* sep = ""; @@ -343,6 +345,8 @@ void UnorderedElementsAreMatcherImplBase::DescribeNegationToImpl( case UnorderedMatcherRequire::Subset: *os << "no injection from elements to requirements exists such that:\n"; break; + default: + break; } const char* sep = ""; for (size_t i = 0; i != matcher_describers_.size(); ++i) { diff --git a/googlemock/src/gmock-spec-builders.cc b/googlemock/src/gmock-spec-builders.cc index 603ad7aa0f..701a57b3ef 100644 --- a/googlemock/src/gmock-spec-builders.cc +++ b/googlemock/src/gmock-spec-builders.cc @@ -301,6 +301,7 @@ void ReportUninterestingCall(CallReaction reaction, const std::string& msg) { "knowing-when-to-expect-useoncall for details.\n", stack_frames_to_skip); break; + case kFail: default: // FAIL Expect(false, nullptr, -1, msg); } diff --git a/googlemock/test/gmock-function-mocker_test.cc b/googlemock/test/gmock-function-mocker_test.cc index c3719f9a30..8160925106 100644 --- a/googlemock/test/gmock-function-mocker_test.cc +++ b/googlemock/test/gmock-function-mocker_test.cc @@ -73,7 +73,7 @@ class TemplatedCopyable { TemplatedCopyable() = default; template - TemplatedCopyable(const U& other) {} // NOLINT + TemplatedCopyable(const U&) {} // NOLINT }; class FooInterface { diff --git a/googlemock/test/gmock-matchers-containers_test.cc b/googlemock/test/gmock-matchers-containers_test.cc index c3b3ef03ff..9b2f001ee6 100644 --- a/googlemock/test/gmock-matchers-containers_test.cc +++ b/googlemock/test/gmock-matchers-containers_test.cc @@ -2829,6 +2829,8 @@ class PredicateFormatterFromMatcherTest : public ::testing::Test { // an "interested" listener; so this will return |true|, thus // simulating a flaky matcher. return listener->IsInterested(); + default: + break; } GTEST_LOG_(FATAL) << "This should never be reached"; diff --git a/googletest/cmake/internal_utils.cmake b/googletest/cmake/internal_utils.cmake index 7ca256a76b..6788511164 100644 --- a/googletest/cmake/internal_utils.cmake +++ b/googletest/cmake/internal_utils.cmake @@ -91,7 +91,17 @@ macro(config_compiler_and_linker) endif() elseif (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "IntelLLVM") - set(cxx_base_flags "-Wall -Wshadow -Wconversion -Wundef") + set(cxx_base_flags "-Wall \ + -Wshadow \ + -Wconversion \ + -Wundef \ + -Wswitch-default \ + -Wswitch-enum \ + -Wextra \ + -Wcast-align \ + -Wunused \ + -Wunreachable-code \ + ") set(cxx_exception_flags "-fexceptions") set(cxx_no_exception_flags "-fno-exceptions") set(cxx_strict_flags "-W -Wpointer-arith -Wreturn-type -Wcast-qual -Wwrite-strings -Wswitch -Wunused-parameter -Wcast-align -Winline -Wredundant-decls") diff --git a/googletest/include/gtest/internal/gtest-death-test-internal.h b/googletest/include/gtest/internal/gtest-death-test-internal.h index b363259ec6..80afa98075 100644 --- a/googletest/include/gtest/internal/gtest-death-test-internal.h +++ b/googletest/include/gtest/internal/gtest-death-test-internal.h @@ -243,6 +243,8 @@ GTEST_API_ bool ExitedUnsuccessfully(int exit_status); gtest_dt->Abort(::testing::internal::DeathTest::TEST_DID_NOT_DIE); \ break; \ } \ + default: \ + break; \ } \ } \ } else \ diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc index cd218c9b0b..0ee51e4df8 100644 --- a/googletest/src/gtest.cc +++ b/googletest/src/gtest.cc @@ -1471,6 +1471,8 @@ class Hunk { ++adds_; hunk_adds_.push_back(std::make_pair('+', line)); break; + default: + break; } } @@ -3281,6 +3283,7 @@ static const char* GetAnsiColorCode(GTestColor color) { return "2"; case GTestColor::kYellow: return "3"; + case GTestColor::kDefault: default: assert(false); return "9"; @@ -3537,6 +3540,9 @@ void PrettyUnitTestResultPrinter::OnTestPartResult( // If the test part succeeded, we don't need to do anything. case TestPartResult::kSuccess: return; + case TestPartResult::kNonFatalFailure: + case TestPartResult::kFatalFailure: + case TestPartResult::kSkip: default: // Print failure message from the assertion // (e.g. expected this and got that). @@ -3755,6 +3761,9 @@ void BriefUnitTestResultPrinter::OnTestPartResult( // If the test part succeeded, we don't need to do anything. case TestPartResult::kSuccess: return; + case TestPartResult::kNonFatalFailure: + case TestPartResult::kFatalFailure: + case TestPartResult::kSkip: default: // Print failure message from the assertion // (e.g. expected this and got that). diff --git a/googletest/test/googletest-death-test-test.cc b/googletest/test/googletest-death-test-test.cc index 877031538f..28e1557fd1 100644 --- a/googletest/test/googletest-death-test-test.cc +++ b/googletest/test/googletest-death-test-test.cc @@ -346,7 +346,7 @@ TEST_F(TestForDeathTest, SwitchStatement) { ASSERT_DEATH(_Exit(1), "") << "exit in default switch handler"; switch (0) - case 0: + default: EXPECT_DEATH(_Exit(1), "") << "exit in switch case"; GTEST_DISABLE_MSC_WARNINGS_POP_() @@ -1499,7 +1499,7 @@ TEST(ConditionalDeathMacrosSyntaxDeathTest, SwitchStatement) { ASSERT_DEATH_IF_SUPPORTED(_Exit(1), "") << "exit in default switch handler"; switch (0) - case 0: + default: EXPECT_DEATH_IF_SUPPORTED(_Exit(1), "") << "exit in switch case"; GTEST_DISABLE_MSC_WARNINGS_POP_() diff --git a/googletest/test/googletest-port-test.cc b/googletest/test/googletest-port-test.cc index 975b2fc334..0fc50765c6 100644 --- a/googletest/test/googletest-port-test.cc +++ b/googletest/test/googletest-port-test.cc @@ -239,7 +239,7 @@ TEST(GtestCheckSyntaxTest, WorksWithSwitch) { } switch (0) - case 0: + default: GTEST_CHECK_(true) << "Check failed in switch case"; } diff --git a/googletest/test/gtest_environment_test.cc b/googletest/test/gtest_environment_test.cc index 03657c7d89..54334b9f6b 100644 --- a/googletest/test/gtest_environment_test.cc +++ b/googletest/test/gtest_environment_test.cc @@ -63,6 +63,7 @@ class MyEnvironment : public testing::Environment { case FATAL_FAILURE: FAIL() << "Expected fatal failure in global set-up."; break; + case NO_FAILURE: default: break; } diff --git a/googletest/test/gtest_unittest.cc b/googletest/test/gtest_unittest.cc index 3680b918b9..14455242bf 100644 --- a/googletest/test/gtest_unittest.cc +++ b/googletest/test/gtest_unittest.cc @@ -4253,7 +4253,7 @@ TEST(AssertionSyntaxTest, WorksWithSwitch) { } switch (0) - case 0: + default: EXPECT_FALSE(false) << "EXPECT_FALSE failed in switch case"; // Binary assertions are implemented using a different code path @@ -4265,7 +4265,7 @@ TEST(AssertionSyntaxTest, WorksWithSwitch) { } switch (0) - case 0: + default: EXPECT_NE(1, 2); }