From 80b3670e1ace250ae2d50fb42ede6646066eb140 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 24 Jul 2026 10:05:06 -0700 Subject: [PATCH] Maintain constness in assertions & expectations to resolve operator bool() cv-qualifier inconsistency between ASSERT_FALSE/EXPECT_FALSE vs. ASSERT_TRUE/EXPECT_TRUE Currently, these macros do not behave consistently in how they convert objects to bool, adding extra const qualifiers or using operator!(). This causes them to produce unexpected behavior on types such as the following: ``` struct example { operator bool() { throw std::runtime_error("constness not propagated"); } operator bool() const { return ...; } }; ``` This change preserves the qualifiers of the evaluated objects and makes the evaluation strategy uniform across macros. Fixes: #4832 PiperOrigin-RevId: 953424646 Change-Id: I670a72eec11272c3b68d7eb6f739a15da227acde --- .../include/gtest/gtest-assertion-result.h | 37 +++++++++++--- googletest/include/gtest/gtest.h | 9 ++-- .../include/gtest/internal/gtest-internal.h | 6 +-- googletest/test/gtest_unittest.cc | 50 +++++++++++++++++++ 4 files changed, 87 insertions(+), 15 deletions(-) diff --git a/googletest/include/gtest/gtest-assertion-result.h b/googletest/include/gtest/gtest-assertion-result.h index 7a5e223fa..a72ac9391 100644 --- a/googletest/include/gtest/gtest-assertion-result.h +++ b/googletest/include/gtest/gtest-assertion-result.h @@ -158,13 +158,18 @@ class GTEST_API_ [[nodiscard]] AssertionResult { // The second parameter prevents this overload from being considered if // the argument is implicitly convertible to AssertionResult. In that case // we want AssertionResult's copy constructor to be used. - template - explicit AssertionResult( - const T& success, - std::enable_if_t>* - /*enabler*/ - = nullptr) - : success_(success) {} + template && + !std::is_trivially_constructible_v, + int> = 0> + explicit AssertionResult(T&& success) : success_(std::forward(success)) {} + + // Similar to the mutable overload, but for cases where mutability is + // unnecessary or problematic (e.g., bitfields). + template , + int> = 0> + explicit AssertionResult(const T& success) : success_(success) {} #if defined(_MSC_VER) && (_MSC_VER < 1910 || _MSC_VER >= 1920) GTEST_DISABLE_MSC_WARNINGS_POP_() @@ -226,6 +231,24 @@ class GTEST_API_ [[nodiscard]] AssertionResult { std::unique_ptr< ::std::string> message_; }; +namespace internal { + +// A pair containing the result that an assertion is evaluating, and the +// expected result (true, false). +// +// Contains a conversion operator that indicates whether the two match. +struct AssertionResultExpectation { + testing::AssertionResult assertion_result; + bool expected_result; + + explicit operator bool() const { + bool converted(assertion_result); + return converted == expected_result; + } +}; + +} // namespace internal + // Makes a successful assertion result. GTEST_API_ AssertionResult AssertionSuccess(); diff --git a/googletest/include/gtest/gtest.h b/googletest/include/gtest/gtest.h index a7193379b..089b10e9f 100644 --- a/googletest/include/gtest/gtest.h +++ b/googletest/include/gtest/gtest.h @@ -1818,14 +1818,13 @@ class [[nodiscard]] TestWithParam : public Test, #define GTEST_EXPECT_TRUE(condition) \ GTEST_TEST_BOOLEAN_(condition, #condition, false, true, \ GTEST_NONFATAL_FAILURE_) -#define GTEST_EXPECT_FALSE(condition) \ - GTEST_TEST_BOOLEAN_(!(condition), #condition, true, false, \ +#define GTEST_EXPECT_FALSE(condition) \ + GTEST_TEST_BOOLEAN_(condition, #condition, true, false, \ GTEST_NONFATAL_FAILURE_) #define GTEST_ASSERT_TRUE(condition) \ GTEST_TEST_BOOLEAN_(condition, #condition, false, true, GTEST_FATAL_FAILURE_) -#define GTEST_ASSERT_FALSE(condition) \ - GTEST_TEST_BOOLEAN_(!(condition), #condition, true, false, \ - GTEST_FATAL_FAILURE_) +#define GTEST_ASSERT_FALSE(condition) \ + GTEST_TEST_BOOLEAN_(condition, #condition, true, false, GTEST_FATAL_FAILURE_) // Define these macros to 1 to omit the definition of the corresponding // EXPECT or ASSERT, which clashes with some users' own code. diff --git a/googletest/include/gtest/internal/gtest-internal.h b/googletest/include/gtest/internal/gtest-internal.h index 8f66d1b35..2b048c5dc 100644 --- a/googletest/include/gtest/internal/gtest-internal.h +++ b/googletest/include/gtest/internal/gtest-internal.h @@ -1453,12 +1453,12 @@ class [[nodiscard]] NeverThrown { // representation of expression as it was passed into the EXPECT_TRUE. #define GTEST_TEST_BOOLEAN_(expression, text, actual, expected, fail) \ GTEST_AMBIGUOUS_ELSE_BLOCKER_ \ - if (const ::testing::AssertionResult gtest_ar_ = \ - ::testing::AssertionResult(expression)) \ + if (::testing::internal::AssertionResultExpectation gtest_are_ = { \ + ::testing::AssertionResult(expression), expected}) \ ; \ else \ fail(::testing::internal::GetBoolAssertionFailureMessage( \ - gtest_ar_, text, #actual, #expected)) + gtest_are_.assertion_result, text, #actual, #expected)) #define GTEST_TEST_NO_FATAL_FAILURE_(statement, fail) \ GTEST_AMBIGUOUS_ELSE_BLOCKER_ \ diff --git a/googletest/test/gtest_unittest.cc b/googletest/test/gtest_unittest.cc index c753c3602..081a6e3ce 100644 --- a/googletest/test/gtest_unittest.cc +++ b/googletest/test/gtest_unittest.cc @@ -88,6 +88,20 @@ static_assert(sizeof(decltype(std::declval() << 1)(*)()) > 0, "error in operator<< overload resolution"); +namespace { + +template +struct ConvertibleToBool { + explicit operator bool() { return MutableResult; } + explicit operator bool() const { return ConstResult; } +}; + +struct Bitfield { + bool bit : 1; +}; + +} // namespace + namespace testing { namespace internal { @@ -3699,6 +3713,15 @@ TEST(AssertionTest, AppendUserMessage) { TEST(AssertionTest, ASSERT_TRUE) { ASSERT_TRUE(2 > 1); // NOLINT EXPECT_FATAL_FAILURE(ASSERT_TRUE(2 < 1), "2 < 1"); + + ASSERT_TRUE((ConvertibleToBool())); + ASSERT_TRUE((std::add_const_t>())); + + Bitfield bf = {true}; + ASSERT_TRUE(bf.bit); // & + ASSERT_TRUE(Bitfield{true}.bit); // && + ASSERT_TRUE(static_cast(Bitfield{true}).bit); // const& + ASSERT_TRUE(static_cast(Bitfield{true}).bit); // const&& } // Tests ASSERT_TRUE(predicate) for predicates returning AssertionResult. @@ -3725,6 +3748,15 @@ TEST(AssertionTest, ASSERT_FALSE) { "Value of: 2 > 1\n" " Actual: true\n" "Expected: false"); + + ASSERT_FALSE((ConvertibleToBool())); + ASSERT_FALSE((std::add_const_t>())); + + Bitfield bf = {false}; + ASSERT_FALSE(bf.bit); // & + ASSERT_FALSE(Bitfield{false}.bit); // && + ASSERT_FALSE(static_cast(Bitfield{false}).bit); // const& + ASSERT_FALSE(static_cast(Bitfield{false}).bit); // const&& } // Tests ASSERT_FALSE(predicate) for predicates returning AssertionResult. @@ -4426,6 +4458,15 @@ TEST(ExpectTest, EXPECT_TRUE) { " Actual: false\n" "Expected: true"); EXPECT_NONFATAL_FAILURE(EXPECT_TRUE(2 > 3), "2 > 3"); + + EXPECT_TRUE((ConvertibleToBool())); + EXPECT_TRUE((std::add_const_t>())); + + Bitfield bf = {true}; + EXPECT_TRUE(bf.bit); // & + EXPECT_TRUE(Bitfield{true}.bit); // && + EXPECT_TRUE(static_cast(Bitfield{true}).bit); // const& + EXPECT_TRUE(static_cast(Bitfield{true}).bit); // const&& } // Tests EXPECT_TRUE(predicate) for predicates returning AssertionResult. @@ -4455,6 +4496,15 @@ TEST(ExpectTest, EXPECT_FALSE) { " Actual: true\n" "Expected: false"); EXPECT_NONFATAL_FAILURE(EXPECT_FALSE(2 < 3), "2 < 3"); + + EXPECT_FALSE((ConvertibleToBool())); + EXPECT_FALSE((std::add_const_t>())); + + Bitfield bf = {false}; + EXPECT_FALSE(bf.bit); // & + EXPECT_FALSE(Bitfield{false}.bit); // && + EXPECT_FALSE(static_cast(Bitfield{false}).bit); // const& + EXPECT_FALSE(static_cast(Bitfield{false}).bit); // const&& } // Tests EXPECT_FALSE(predicate) for predicates returning AssertionResult.