Fix CHECK_OP implementation in Release builds

In this particular case, we just did a (lhs)op(rhs), ignoring the case
that lhs and rhs might have different signedness.
This CL changes that to use the proper Cmp##op##Impl implementation,
which does two comparisions for signed-vs-unsigned checks, avoiding
compiler errors.

R=ishell@chromium.org

Review-Url: https://codereview.chromium.org/2642383002
Cr-Commit-Position: refs/heads/master@{#42566}

src/base/logging.h

@@ -43,13 +43,13 @@ namespace base {
 //
 // We make sure CHECK et al. always evaluates their arguments, as
 // doing CHECK(FunctionWithSideEffect()) is a common idiom.
-#define CHECK(condition)                                             \
+#define CHECK_WITH_MSG(condition, message)                        \
   do {                                                            \
     if (V8_UNLIKELY(!(condition))) {                              \
-      V8_Fatal(__FILE__, __LINE__, "Check failed: %s.", #condition); \
+      V8_Fatal(__FILE__, __LINE__, "Check failed: %s.", message); \
     }                                                             \
   } while (0)
-
+#define CHECK(condition) CHECK_WITH_MSG(condition, #condition)
 
 #ifdef DEBUG
 
@@ -70,7 +70,12 @@ namespace base {
 // Make all CHECK functions discard their log strings to reduce code
 // bloat for official release builds.
 
-#define CHECK_OP(name, op, lhs, rhs) CHECK((lhs)op(rhs))
+#define CHECK_OP(name, op, lhs, rhs)                                         \
+  do {                                                                       \
+    bool _cmp =                                                              \
+        ::v8::base::Cmp##name##Impl<decltype(lhs), decltype(rhs)>(lhs, rhs); \
+    CHECK_WITH_MSG(_cmp, #lhs " " #op " " #rhs);                             \
+  } while (0)
 
 #endif
 
@@ -199,7 +204,8 @@ DEFINE_CHECK_OP_IMPL(GT, > )
 #define CHECK_GT(lhs, rhs) CHECK_OP(GT, >, lhs, rhs)
 #define CHECK_NULL(val) CHECK((val) == nullptr)
 #define CHECK_NOT_NULL(val) CHECK((val) != nullptr)
-#define CHECK_IMPLIES(lhs, rhs) CHECK(!(lhs) || (rhs))
+#define CHECK_IMPLIES(lhs, rhs) \
+  CHECK_WITH_MSG(!(lhs) || (rhs), #lhs " implies " #rhs)
 
 }  // namespace base
 }  // namespace v8

test/unittests/base/logging-unittest.cc

@@ -63,5 +63,24 @@ TEST(LoggingTest, CompareAgainstStaticConstPointer) {
   CHECK_SUCCEED(GT, 0, v8::internal::Smi::kMinValue);
 }
 
+TEST(LoggingTest, CompareWithDifferentSignedness) {
+#define CHECK_BOTH(name, lhs, rhs) \
+  CHECK_##name(lhs, rhs);          \
+  DCHECK_##name(lhs, rhs)
+
+  int32_t i32 = 10;
+  uint32_t u32 = 20;
+  int64_t i64 = 30;
+  uint64_t u64 = 40;
+
+  // All these checks should compile (!) and succeed.
+  CHECK_BOTH(EQ, i32 + 10, u32);
+  CHECK_BOTH(LT, i32, u64);
+  CHECK_BOTH(LE, u32, i64);
+  CHECK_BOTH(IMPLIES, i32, i64);
+  CHECK_BOTH(IMPLIES, u32, i64);
+  CHECK_BOTH(IMPLIES, !u32, !i64);
+}
+
 }  // namespace base
 }  // namespace v8