From 93ac3a4f9ee7792af399cebd873ee99ce15aed08 Mon Sep 17 00:00:00 2001
From: Abseil Team <absl-team@google.com>
Date: Thu, 16 May 2024 01:34:16 -0700
Subject: Support `AbslStringify` with `DCHECK_EQ`.
`AbslStringify` is the [recommended](abseil.io/tips/215) way to make a type printable. However, the simple expression `DCHECK_EQ(x, y)` fails when either argument implements it, and not `operator<<`.
PiperOrigin-RevId: 634261367
Change-Id: Ic42666c286cf172c9482abbd28194da828706c71
---
absl/log/BUILD.bazel | 2 +
absl/log/CMakeLists.txt | 4 ++
absl/log/check_test_impl.inc | 158 ++++++++++++++++++++++++++++++++++++++++++
absl/log/internal/check_op.cc | 20 ++++++
absl/log/internal/check_op.h | 70 +++++++++++++++----
5 files changed, 240 insertions(+), 14 deletions(-)
diff --git a/absl/log/BUILD.bazel b/absl/log/BUILD.bazel
index dd192a10..b13cf4d4 100644
--- a/absl/log/BUILD.bazel
+++ b/absl/log/BUILD.bazel
@@ -357,6 +357,8 @@ cc_library(
"//absl/base:core_headers",
"//absl/log/internal:test_helpers",
"//absl/status",
+ "//absl/strings",
+ "//absl/strings:string_view",
"@com_google_googletest//:gtest",
],
)
diff --git a/absl/log/CMakeLists.txt b/absl/log/CMakeLists.txt
index 59aa19f3..43844651 100644
--- a/absl/log/CMakeLists.txt
+++ b/absl/log/CMakeLists.txt
@@ -789,6 +789,8 @@ absl_cc_test(
absl::core_headers
absl::log_internal_test_helpers
absl::status
+ absl::strings
+ absl::string_view
GTest::gmock_main
)
@@ -832,6 +834,8 @@ absl_cc_test(
absl::core_headers
absl::log_internal_test_helpers
absl::status
+ absl::strings
+ absl::string_view
GTest::gmock_main
)
diff --git a/absl/log/check_test_impl.inc b/absl/log/check_test_impl.inc
index d5c0aee4..64318108 100644
--- a/absl/log/check_test_impl.inc
+++ b/absl/log/check_test_impl.inc
@@ -31,6 +31,8 @@
#include "absl/base/config.h"
#include "absl/log/internal/test_helpers.h"
#include "absl/status/status.h"
+#include "absl/strings/string_view.h"
+#include "absl/strings/substitute.h"
// NOLINTBEGIN(misc-definitions-in-headers)
@@ -521,6 +523,162 @@ TEST(CHECKDeathTest, TestUserDefinedStreaming) {
"Check failed: v1 == v2 (ComparableType{1} vs. ComparableType{2})"));
}
+// A type that can be printed using AbslStringify.
+struct StringifiableType {
+ int x = 0;
+ explicit StringifiableType(int x) : x(x) {}
+ friend bool operator==(const StringifiableType& lhs,
+ const StringifiableType& rhs) {
+ return lhs.x == rhs.x;
+ }
+ friend bool operator!=(const StringifiableType& lhs,
+ const StringifiableType& rhs) {
+ return lhs.x != rhs.x;
+ }
+ friend bool operator<(const StringifiableType& lhs,
+ const StringifiableType& rhs) {
+ return lhs.x < rhs.x;
+ }
+ friend bool operator>(const StringifiableType& lhs,
+ const StringifiableType& rhs) {
+ return lhs.x > rhs.x;
+ }
+ friend bool operator<=(const StringifiableType& lhs,
+ const StringifiableType& rhs) {
+ return lhs.x <= rhs.x;
+ }
+ friend bool operator>=(const StringifiableType& lhs,
+ const StringifiableType& rhs) {
+ return lhs.x >= rhs.x;
+ }
+ template <typename Sink>
+ friend void AbslStringify(Sink& sink, const StringifiableType& obj) {
+ absl::Format(&sink, "StringifiableType{%d}", obj.x);
+ }
+
+ // Make sure no unintended copy happens.
+ StringifiableType(const StringifiableType&) = delete;
+};
+
+TEST(CHECKTest, TestUserDefinedAbslStringify) {
+ const StringifiableType v1(1);
+ const StringifiableType v2(2);
+
+ ABSL_TEST_CHECK_EQ(v1, v1);
+ ABSL_TEST_CHECK_NE(v1, v2);
+ ABSL_TEST_CHECK_LT(v1, v2);
+ ABSL_TEST_CHECK_LE(v1, v2);
+ ABSL_TEST_CHECK_GT(v2, v1);
+ ABSL_TEST_CHECK_GE(v2, v1);
+}
+
+TEST(CHECKDeathTest, TestUserDefinedAbslStringify) {
+ const StringifiableType v1(1);
+ const StringifiableType v2(2);
+
+ // Returns a matcher for the expected check failure message when comparing two
+ // values.
+ auto expected_output = [](int lhs, absl::string_view condition, int rhs) {
+ return HasSubstr(
+ absl::Substitute("Check failed: v$0 $1 v$2 (StringifiableType{$0} vs. "
+ "StringifiableType{$2})",
+ lhs, condition, rhs));
+ };
+ // Test comparisons where the check fails.
+ EXPECT_DEATH(ABSL_TEST_CHECK_EQ(v1, v2), expected_output(1, "==", 2));
+ EXPECT_DEATH(ABSL_TEST_CHECK_NE(v1, v1), expected_output(1, "!=", 1));
+ EXPECT_DEATH(ABSL_TEST_CHECK_LT(v2, v1), expected_output(2, "<", 1));
+ EXPECT_DEATH(ABSL_TEST_CHECK_LE(v2, v1), expected_output(2, "<=", 1));
+ EXPECT_DEATH(ABSL_TEST_CHECK_GT(v1, v2), expected_output(1, ">", 2));
+ EXPECT_DEATH(ABSL_TEST_CHECK_GE(v1, v2), expected_output(1, ">=", 2));
+}
+
+// A type that can be printed using both AbslStringify and operator<<.
+struct StringifiableStreamableType {
+ int x = 0;
+ explicit StringifiableStreamableType(int x) : x(x) {}
+
+ friend bool operator==(const StringifiableStreamableType& lhs,
+ const StringifiableStreamableType& rhs) {
+ return lhs.x == rhs.x;
+ }
+ friend bool operator!=(const StringifiableStreamableType& lhs,
+ const StringifiableStreamableType& rhs) {
+ return lhs.x != rhs.x;
+ }
+ template <typename Sink>
+ friend void AbslStringify(Sink& sink,
+ const StringifiableStreamableType& obj) {
+ absl::Format(&sink, "Strigified{%d}", obj.x);
+ }
+ friend std::ostream& operator<<(std::ostream& out,
+ const StringifiableStreamableType& obj) {
+ return out << "Streamed{" << obj.x << "}";
+ }
+
+ // Avoid unintentional copy.
+ StringifiableStreamableType(const StringifiableStreamableType&) = delete;
+};
+
+TEST(CHECKDeathTest, TestStreamingPreferredOverAbslStringify) {
+ StringifiableStreamableType v1(1);
+ StringifiableStreamableType v2(2);
+
+ EXPECT_DEATH(
+ ABSL_TEST_CHECK_EQ(v1, v2),
+ HasSubstr("Check failed: v1 == v2 (Streamed{1} vs. Streamed{2})"));
+}
+
+// A type whose pointer can be passed to AbslStringify.
+struct PointerIsStringifiable {};
+template <typename Sink>
+void AbslStringify(Sink& sink, const PointerIsStringifiable* var) {
+ sink.Append("PointerIsStringifiable");
+}
+
+// Verifies that a pointer is printed as a number despite having AbslStringify
+// defined. Users may implement AbslStringify that dereferences the pointer, and
+// doing so as part of DCHECK would not be good.
+TEST(CHECKDeathTest, TestPointerPrintedAsNumberDespiteAbslStringify) {
+ const auto* p = reinterpret_cast<const PointerIsStringifiable*>(0x1234);
+
+#ifdef _MSC_VER
+ EXPECT_DEATH(
+ ABSL_TEST_CHECK_EQ(p, nullptr),
+ HasSubstr("Check failed: p == nullptr (0000000000001234 vs. (null))"));
+#else // _MSC_VER
+ EXPECT_DEATH(ABSL_TEST_CHECK_EQ(p, nullptr),
+ HasSubstr("Check failed: p == nullptr (0x1234 vs. (null))"));
+#endif // _MSC_VER
+}
+
+// An uncopyable object with operator<<.
+struct Uncopyable {
+ int x;
+ explicit Uncopyable(int x) : x(x) {}
+ Uncopyable(const Uncopyable&) = delete;
+ friend bool operator==(const Uncopyable& lhs, const Uncopyable& rhs) {
+ return lhs.x == rhs.x;
+ }
+ friend bool operator!=(const Uncopyable& lhs, const Uncopyable& rhs) {
+ return lhs.x != rhs.x;
+ }
+ friend std::ostream& operator<<(std::ostream& os, const Uncopyable& obj) {
+ return os << "Uncopyable{" << obj.x << "}";
+ }
+};
+
+// Test that an uncopyable object can be used.
+// Will catch us if implementation has an unintended copy.
+TEST(CHECKDeathTest, TestUncopyable) {
+ const Uncopyable v1(1);
+ const Uncopyable v2(2);
+
+ EXPECT_DEATH(
+ ABSL_TEST_CHECK_EQ(v1, v2),
+ HasSubstr("Check failed: v1 == v2 (Uncopyable{1} vs. Uncopyable{2})"));
+}
+
} // namespace absl_log_internal
// NOLINTEND(misc-definitions-in-headers)
diff --git a/absl/log/internal/check_op.cc b/absl/log/internal/check_op.cc
index f4b67647..23c4a3b2 100644
--- a/absl/log/internal/check_op.cc
+++ b/absl/log/internal/check_op.cc
@@ -16,6 +16,10 @@
#include <string.h>
+#include <ostream>
+
+#include "absl/strings/string_view.h"
+
#ifdef _MSC_VER
#define strcasecmp _stricmp
#else
@@ -113,6 +117,22 @@ DEFINE_CHECK_STROP_IMPL(CHECK_STRCASEEQ, strcasecmp, true)
DEFINE_CHECK_STROP_IMPL(CHECK_STRCASENE, strcasecmp, false)
#undef DEFINE_CHECK_STROP_IMPL
+namespace detect_specialization {
+
+StringifySink::StringifySink(std::ostream& os) : os_(os) {}
+
+void StringifySink::Append(absl::string_view text) { os_ << text; }
+
+void StringifySink::Append(size_t length, char ch) {
+ for (size_t i = 0; i < length; ++i) os_.put(ch);
+}
+
+void AbslFormatFlush(StringifySink* sink, absl::string_view text) {
+ sink->Append(text);
+}
+
+} // namespace detect_specialization
+
} // namespace log_internal
ABSL_NAMESPACE_END
} // namespace absl
diff --git a/absl/log/internal/check_op.h b/absl/log/internal/check_op.h
index 0d24f4d1..21592207 100644
--- a/absl/log/internal/check_op.h
+++ b/absl/log/internal/check_op.h
@@ -24,9 +24,11 @@
#include <stdint.h>
+#include <cstddef>
#include <ostream>
#include <sstream>
#include <string>
+#include <type_traits>
#include <utility>
#include "absl/base/attributes.h"
@@ -35,6 +37,8 @@
#include "absl/log/internal/nullguard.h"
#include "absl/log/internal/nullstream.h"
#include "absl/log/internal/strip.h"
+#include "absl/strings/has_absl_stringify.h"
+#include "absl/strings/string_view.h"
// `ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL` wraps string literals that
// should be stripped when `ABSL_MIN_LOG_LEVEL` exceeds `kFatal`.
@@ -287,6 +291,44 @@ decltype(detect_specialization::operator<<(std::declval<std::ostream&>(),
std::declval<const T&>()))
Detect(char);
+// A sink for AbslStringify which redirects everything to a std::ostream.
+class StringifySink {
+ public:
+ explicit StringifySink(std::ostream& os ABSL_ATTRIBUTE_LIFETIME_BOUND);
+
+ void Append(absl::string_view text);
+ void Append(size_t length, char ch);
+ friend void AbslFormatFlush(StringifySink* sink, absl::string_view text);
+
+ private:
+ std::ostream& os_;
+};
+
+// Wraps a type implementing AbslStringify, and implements operator<<.
+template <typename T>
+class StringifyToStreamWrapper {
+ public:
+ explicit StringifyToStreamWrapper(const T& v ABSL_ATTRIBUTE_LIFETIME_BOUND)
+ : v_(v) {}
+
+ friend std::ostream& operator<<(std::ostream& os,
+ const StringifyToStreamWrapper& wrapper) {
+ StringifySink sink(os);
+ AbslStringify(sink, wrapper.v_);
+ return os;
+ }
+
+ private:
+ const T& v_;
+};
+
+// This overload triggers when T implements AbslStringify.
+// StringifyToStreamWrapper is used to allow MakeCheckOpString to use
+// operator<<.
+template <typename T>
+std::enable_if_t<HasAbslStringify<T>::value,
+ StringifyToStreamWrapper<T>>
+Detect(...); // Ellipsis has lowest preference when int passed.
} // namespace detect_specialization
template <typename T>
@@ -342,20 +384,20 @@ ABSL_LOG_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING_EXTERN(const void*);
// `(int, int)` override works around the issue that the compiler will not
// instantiate the template version of the function on values of unnamed enum
// type.
-#define ABSL_LOG_INTERNAL_CHECK_OP_IMPL(name, op) \
- template <typename T1, typename T2> \
- inline constexpr ::std::string* name##Impl(const T1& v1, const T2& v2, \
- const char* exprtext) { \
- using U1 = CheckOpStreamType<T1>; \
- using U2 = CheckOpStreamType<T2>; \
- return ABSL_PREDICT_TRUE(v1 op v2) \
- ? nullptr \
- : ABSL_LOG_INTERNAL_CHECK_OP_IMPL_RESULT(U1, U2, v1, v2, \
- exprtext); \
- } \
- inline constexpr ::std::string* name##Impl(int v1, int v2, \
- const char* exprtext) { \
- return name##Impl<int, int>(v1, v2, exprtext); \
+#define ABSL_LOG_INTERNAL_CHECK_OP_IMPL(name, op) \
+ template <typename T1, typename T2> \
+ inline constexpr ::std::string* name##Impl(const T1& v1, const T2& v2, \
+ const char* exprtext) { \
+ using U1 = CheckOpStreamType<T1>; \
+ using U2 = CheckOpStreamType<T2>; \
+ return ABSL_PREDICT_TRUE(v1 op v2) \
+ ? nullptr \
+ : ABSL_LOG_INTERNAL_CHECK_OP_IMPL_RESULT(U1, U2, U1(v1), \
+ U2(v2), exprtext); \
+ } \
+ inline constexpr ::std::string* name##Impl(int v1, int v2, \
+ const char* exprtext) { \
+ return name##Impl<int, int>(v1, v2, exprtext); \
}
ABSL_LOG_INTERNAL_CHECK_OP_IMPL(Check_EQ, ==)
--
cgit v1.2.3