logging: add XCHECK_EQ and friends to XCHECK and XDCHECK

Summary:
Add XCHECK_EQ(), XCHECK_NE(), XCHECK_LT(), XCHECK_LE(), XCHECK_GT(), and
XCHECK_GE(), plus corresponding XDCHECK_*() versions that only fail in debug
buildk.

These are similar to XCHECK()/XDCHECK(), but on failure they also log the
values of the two expressions being compared.

Reviewed By: therealgymmy

Differential Revision: D14016494

fbshipit-source-id: dba02d748f78d306227ec734d9e52ef0bc73919b

folly/logging/test/XlogTest.cpp

@@ -406,3 +406,13 @@ TEST(Xlog, xlogStripFilename) {
           "src/test.cpp") == 0,
       "incorrect xlogStripFilename() behavior");
 }
+
+TEST(Xlog, XCheckPrecedence) {
+  // Ensure that XCHECK_XX() and XDCHECK_XX() avoid the common macro pitfall of
+  // not wrapping arguments in parentheses and causing incorrect operator
+  // precedence issues.
+  XCHECK_EQ(0x22 & 0x3, 2);
+  XDCHECK_EQ(2, 0x22 & 0x3);
+  XCHECK_NE(0x62 & 0x22, 2);
+  XDCHECK_NE(0x62 & 0x22, 2);
+}

folly/logging/test/fatal_test.py

@@ -180,3 +180,50 @@ class FatalTests(unittest.TestCase):
         else:
             self.assertEqual(b"", err)
             self.assertEqual(0, returncode)
+
+    def test_xcheck_comparisons(self):
+        self._test_xcheck_cmp("xcheck_eq0", 1, "==")
+        self._test_xcheck_cmp("xcheck_ne0", 0, "!=")
+        self._test_xcheck_cmp("xcheck_lt0", 0, "<")
+        self._test_xcheck_cmp("xcheck_le0", 9, "<=")
+        self._test_xcheck_cmp("xcheck_gt0", 0, ">")
+        self._test_xcheck_cmp("xcheck_ge0", -3, ">=")
+
+        self._test_xcheck_cmp("xcheck_eq0", 0, "==", expect_failure=False)
+        self._test_xcheck_cmp("xcheck_ne0", 123, "!=", expect_failure=False)
+        self._test_xcheck_cmp("xcheck_lt0", -12, "<", expect_failure=False)
+        self._test_xcheck_cmp("xcheck_le0", 0, "<=", expect_failure=False)
+        self._test_xcheck_cmp("xcheck_gt0", 123, ">", expect_failure=False)
+        self._test_xcheck_cmp("xcheck_ge0", 123, ">=", expect_failure=False)
+
+        is_debug = self.is_debug_build()
+        self._test_xcheck_cmp("xdcheck_eq0", 1, "==", expect_failure=is_debug)
+        self._test_xcheck_cmp("xdcheck_ne0", 0, "!=", expect_failure=is_debug)
+        self._test_xcheck_cmp("xdcheck_lt0", 0, "<", expect_failure=is_debug)
+        self._test_xcheck_cmp("xdcheck_le0", 9, "<=", expect_failure=is_debug)
+        self._test_xcheck_cmp("xdcheck_gt0", 0, ">", expect_failure=is_debug)
+        self._test_xcheck_cmp("xdcheck_ge0", -3, ">=", expect_failure=is_debug)
+
+    def test_xcheck_eval_once(self):
+        err = self.run_helper("--test_xcheck_eq_evalutates_once")
+        expected_msg = "Check failed: ++x == 7 (6 vs. 7)"
+        self.assertRegex(err, self.get_crash_regex(expected_msg.encode("utf-8")))
+
+    def _test_xcheck_cmp(
+        self, flag, value, op, extra_msg=" extra user args", expect_failure=True
+    ):
+        args = ["--crash=no", "--" + flag, str(value)]
+        if expect_failure:
+            err = self.run_helper(*args)
+            expected_msg = "Check failed: FLAGS_%s %s 0 (%s vs. 0)%s" % (
+                flag,
+                op,
+                value,
+                extra_msg,
+            )
+            self.assertRegex(err, self.get_crash_regex(expected_msg.encode("utf-8")))
+        else:
+            returncode, out, err = self.run_helper_nochecks(*args)
+            self.assertEqual(b"", err)
+            self.assertEqual(b"", out)
+            self.assertEqual(0, returncode)

folly/logging/test/helpers/FatalHelper.cpp

@@ -38,6 +38,25 @@ DEFINE_bool(
     "Fail an XCHECK() test with no additional message.");
 DEFINE_bool(fail_xdcheck, false, "Fail an XDCHECK() test.");
 
+DEFINE_int32(xcheck_eq0, 0, "Check this value using XCHECK_EQ(value, 0)");
+DEFINE_int32(xcheck_ne0, 1, "Check this value using XCHECK_NE 0)");
+DEFINE_int32(xcheck_lt0, -1, "Check this value using XCHECK_LT(value, 0)");
+DEFINE_int32(xcheck_le0, 0, "Check this value using XCHECK_LE(value, 0)");
+DEFINE_int32(xcheck_gt0, 1, "Check this value using XCHECK_GT(value, 0)");
+DEFINE_int32(xcheck_ge0, 0, "Check this value using XCHECK_GE(value, 0)");
+
+DEFINE_int32(xdcheck_eq0, 0, "Check this value using XDCHECK_EQ(value, 0)");
+DEFINE_int32(xdcheck_ne0, 1, "Check this value using XDCHECK_NE 0)");
+DEFINE_int32(xdcheck_lt0, -1, "Check this value using XDCHECK_LT(value, 0)");
+DEFINE_int32(xdcheck_le0, 0, "Check this value using XDCHECK_LE(value, 0)");
+DEFINE_int32(xdcheck_gt0, 1, "Check this value using XDCHECK_GT(value, 0)");
+DEFINE_int32(xdcheck_ge0, 0, "Check this value using XDCHECK_GE(value, 0)");
+
+DEFINE_bool(
+    test_xcheck_eq_evalutates_once,
+    false,
+    "Test an XCHECK_EQ() statement where the arguments have side effects");
+
 using folly::LogLevel;
 
 namespace {
@@ -110,7 +129,27 @@ int main(int argc, char* argv[]) {
   XCHECK(!FLAGS_fail_xcheck_nomsg);
   XDCHECK(!FLAGS_fail_xdcheck) << ": --fail_xdcheck specified!";
 
-  // Do most of the work in a separate helper function.
+  XCHECK_EQ(FLAGS_xcheck_eq0, 0) << " extra user args";
+  XCHECK_NE(FLAGS_xcheck_ne0, 0, " extra user args");
+  XCHECK_LT(FLAGS_xcheck_lt0, 0, " extra ", "user", " args");
+  XCHECK_LE(FLAGS_xcheck_le0, 0, " extra ", "user") << " args";
+  XCHECK_GT(FLAGS_xcheck_gt0, 0) << " extra user args";
+  XCHECK_GE(FLAGS_xcheck_ge0, 0) << " extra user args";
+  XDCHECK_EQ(FLAGS_xdcheck_eq0, 0) << " extra user args";
+  XDCHECK_NE(FLAGS_xdcheck_ne0, 0, " extra user args");
+  XDCHECK_LT(FLAGS_xdcheck_lt0, 0) << " extra user args";
+  XDCHECK_LE(FLAGS_xdcheck_le0, 0) << " extra user args";
+  XDCHECK_GT(FLAGS_xdcheck_gt0, 0) << " extra user args";
+  XDCHECK_GE(FLAGS_xdcheck_ge0, 0) << " extra user args";
+
+  if (FLAGS_test_xcheck_eq_evalutates_once) {
+    // Make sure XCHECK_EQ() only evaluates "++x" once,
+    // and logs that it equals 6 and not 7.
+    int x = 5;
+    XCHECK_EQ(++x, 7);
+  }
+
+  // Do the remainder of the work in a separate helper function.
   //
   // The main reason for putting this in a helper function is to ensure that
   // the compiler does not warn about missing return statements on XLOG(FATAL)

folly/logging/xlog.h

@@ -332,12 +332,54 @@
 #define XCHECK(cond, ...) \
   XLOG_IF(FATAL, UNLIKELY(!(cond)), "Check failed: " #cond " ", ##__VA_ARGS__)
 
+namespace folly {
+namespace detail {
+template <typename Arg1, typename Arg2, typename CmpFn>
+std::unique_ptr<std::string> XCheckOpImpl(
+    folly::StringPiece checkStr,
+    const Arg1& arg1,
+    const Arg2& arg2,
+    CmpFn&& cmp_fn) {
+  if (LIKELY(cmp_fn(arg1, arg2))) {
+    return nullptr;
+  }
+  return std::make_unique<std::string>(folly::to<std::string>(
+      "Check failed: ", checkStr, " (", arg1, " vs. ", arg2, ")"));
+}
+} // namespace detail
+} // namespace folly
+
+#define XCHECK_OP(op, arg1, arg2, ...)                                        \
+  while (auto _folly_logging_check_result = ::folly::detail::XCheckOpImpl(    \
+             #arg1 " " #op " " #arg2,                                         \
+             (arg1),                                                          \
+             (arg2),                                                          \
+             [](const auto& _folly_check_arg1, const auto& _folly_check_arg2) \
+                 -> bool { return _folly_check_arg1 op _folly_check_arg2; })) \
+  XLOG(FATAL, *_folly_logging_check_result, ##__VA_ARGS__)
+
+/**
+ * Assert a comparison relationship between two arguments.
+ *
+ * If the comparison check fails the values of both expressions being compared
+ * will be included in the failure message.  This is the main benefit of using
+ * these specific comparison macros over XCHECK().  XCHECK() will simply log
+ * that the expression evaluated was false, while these macros include the
+ * specific values being compared.
+ */
+#define XCHECK_EQ(arg1, arg2, ...) XCHECK_OP(==, arg1, arg2, ##__VA_ARGS__)
+#define XCHECK_NE(arg1, arg2, ...) XCHECK_OP(!=, arg1, arg2, ##__VA_ARGS__)
+#define XCHECK_LT(arg1, arg2, ...) XCHECK_OP(<, arg1, arg2, ##__VA_ARGS__)
+#define XCHECK_GT(arg1, arg2, ...) XCHECK_OP(>, arg1, arg2, ##__VA_ARGS__)
+#define XCHECK_LE(arg1, arg2, ...) XCHECK_OP(<=, arg1, arg2, ##__VA_ARGS__)
+#define XCHECK_GE(arg1, arg2, ...) XCHECK_OP(>=, arg1, arg2, ##__VA_ARGS__)
+
 /**
- * Assert that a condition is true in non-debug builds.
+ * Assert that a condition is true in debug builds only.
  *
- * When NDEBUG is set this behaves like XDCHECK()
- * When NDEBUG is not defined XDCHECK statements are not evaluated and will
- * never log.
+ * When NDEBUG is not defined this behaves like XCHECK().
+ * When NDEBUG is defined XDCHECK statements are not evaluated and will never
+ * log.
  *
  * You can use `XLOG_IF(DFATAL, condition)` instead if you want the condition to
  * be evaluated in release builds but log a message without crashing the
@@ -346,6 +388,34 @@
 #define XDCHECK(cond, ...) \
   (!::folly::kIsDebug) ? static_cast<void>(0) : XCHECK(cond, ##__VA_ARGS__)
 
+/*
+ * It would be nice to rely solely on folly::kIsDebug here rather than NDEBUG.
+ * However doing so would make the code substantially more complicated.  It is
+ * much simpler to simply change the definition of XDCHECK_OP() based on NDEBUG.
+ */
+#ifdef NDEBUG
+#define XDCHECK_OP(op, arg1, arg2, ...) \
+  while (false)                         \
+  XCHECK_OP(op, arg1, arg2, ##__VA_ARGS__)
+#else
+#define XDCHECK_OP(op, arg1, arg2, ...) XCHECK_OP(op, arg1, arg2, ##__VA_ARGS__)
+#endif
+
+/**
+ * Assert a comparison relationship between two arguments in debug builds.
+ *
+ * When NDEBUG is not set these macros behaves like the corresponding
+ * XCHECK_XX() versions (XCHECK_EQ(), XCHECK_NE(), etc).
+ *
+ * When NDEBUG is defined these statements are not evaluated and will never log.
+ */
+#define XDCHECK_EQ(arg1, arg2, ...) XDCHECK_OP(==, arg1, arg2, ##__VA_ARGS__)
+#define XDCHECK_NE(arg1, arg2, ...) XDCHECK_OP(!=, arg1, arg2, ##__VA_ARGS__)
+#define XDCHECK_LT(arg1, arg2, ...) XDCHECK_OP(<, arg1, arg2, ##__VA_ARGS__)
+#define XDCHECK_GT(arg1, arg2, ...) XDCHECK_OP(>, arg1, arg2, ##__VA_ARGS__)
+#define XDCHECK_LE(arg1, arg2, ...) XDCHECK_OP(<=, arg1, arg2, ##__VA_ARGS__)
+#define XDCHECK_GE(arg1, arg2, ...) XDCHECK_OP(>=, arg1, arg2, ##__VA_ARGS__)
+
 /**
  * XLOG_IS_IN_HEADER_FILE evaluates to false if we can definitively tell if we
  * are not in a header file.  Otherwise, it evaluates to true.