Make CHECK_EQ and friends work properly in an if/else structure without braces.
BUG=
Review URL: https://codereview.chromium.org/1193873004
Cr-Commit-Position: refs/heads/master@{#339943}
CrOS-Libchrome-Original-Commit: 6ad937bc698ce8958d6578609919fe9fd05429f6
diff --git a/base/logging.h b/base/logging.h
index adc8fb6..ad9033e 100644
--- a/base/logging.h
+++ b/base/logging.h
@@ -426,6 +426,21 @@
#define EAT_STREAM_PARAMETERS \
true ? (void) 0 : ::logging::LogMessageVoidify() & LOG_STREAM(FATAL)
+// Captures the result of a CHECK_EQ (for example) and facilitates testing as a
+// boolean.
+class CheckOpResult {
+ public:
+ // |message| must be null if and only if the check failed.
+ CheckOpResult(std::string* message) : message_(message) {}
+ // Returns true if the check succeeded.
+ operator bool() const { return !message_; }
+ // Returns the message.
+ std::string* message() { return message_; }
+
+ private:
+ std::string* message_;
+};
+
// CHECK dies with a fatal error if condition is not true. It is *not*
// controlled by NDEBUG, so the check will be executed regardless of
// compilation mode.
@@ -483,14 +498,18 @@
// Helper macro for binary operators.
// Don't use this macro directly in your code, use CHECK_EQ et al below.
-//
-// TODO(akalin): Rewrite this so that constructs like if (...)
-// CHECK_EQ(...) else { ... } work properly.
-#define CHECK_OP(name, op, val1, val2) \
- if (std::string* _result = \
- logging::Check##name##Impl((val1), (val2), \
- #val1 " " #op " " #val2)) \
- logging::LogMessage(__FILE__, __LINE__, _result).stream()
+// The 'switch' is used to prevent the 'else' from being ambiguous when the
+// macro is used in an 'if' clause such as:
+// if (a == 1)
+// CHECK_EQ(2, a);
+#define CHECK_OP(name, op, val1, val2) \
+ switch (0) case 0: default: \
+ if (logging::CheckOpResult true_if_passed = \
+ logging::Check##name##Impl((val1), (val2), \
+ #val1 " " #op " " #val2)) \
+ ; \
+ else \
+ logging::LogMessage(__FILE__, __LINE__, true_if_passed.message()).stream()
#endif
@@ -666,12 +685,20 @@
// Helper macro for binary operators.
// Don't use this macro directly in your code, use DCHECK_EQ et al below.
-#define DCHECK_OP(name, op, val1, val2) \
- if (DCHECK_IS_ON()) \
- if (std::string* _result = logging::Check##name##Impl( \
- (val1), (val2), #val1 " " #op " " #val2)) \
- logging::LogMessage(__FILE__, __LINE__, ::logging::LOG_DCHECK, _result) \
- .stream()
+// The 'switch' is used to prevent the 'else' from being ambiguous when the
+// macro is used in an 'if' clause such as:
+// if (a == 1)
+// DCHECK_EQ(2, a);
+#define DCHECK_OP(name, op, val1, val2) \
+ switch (0) case 0: default: \
+ if (logging::CheckOpResult true_if_passed = \
+ DCHECK_IS_ON() ? \
+ logging::Check##name##Impl((val1), (val2), \
+ #val1 " " #op " " #val2) : nullptr) \
+ ; \
+ else \
+ logging::LogMessage(__FILE__, __LINE__, ::logging::LOG_DCHECK, \
+ true_if_passed.message()).stream()
// Equality/Inequality checks - compare two values, and log a
// LOG_DCHECK message including the two values when the result is not
diff --git a/base/logging_unittest.cc b/base/logging_unittest.cc
index 8b9701a..e061942 100644
--- a/base/logging_unittest.cc
+++ b/base/logging_unittest.cc
@@ -234,6 +234,30 @@
DCHECK_EQ(some_variable, 1) << "test";
}
+TEST_F(LoggingTest, DCheckEqStatements) {
+ bool reached = false;
+ if (false)
+ DCHECK_EQ(false, true); // Unreached.
+ else
+ DCHECK_EQ(true, reached = true); // Reached, passed.
+ ASSERT_EQ(DCHECK_IS_ON() ? true : false, reached);
+
+ if (false)
+ DCHECK_EQ(false, true); // Unreached.
+}
+
+TEST_F(LoggingTest, CheckEqStatements) {
+ bool reached = false;
+ if (false)
+ CHECK_EQ(false, true); // Unreached.
+ else
+ CHECK_EQ(true, reached = true); // Reached, passed.
+ ASSERT_TRUE(reached);
+
+ if (false)
+ CHECK_EQ(false, true); // Unreached.
+}
+
// Test that defining an operator<< for a type in a namespace doesn't prevent
// other code in that namespace from calling the operator<<(ostream, wstring)
// defined by logging.h. This can fail if operator<<(ostream, wstring) can't be