From 1d4854bc87a16e449760dab54e8b0c304323d133 Mon Sep 17 00:00:00 2001
From: Yedidya Feldblum <yfeldblum@fb.com>
Date: Mon, 15 Aug 2016 15:22:00 -0700
Subject: [PATCH] Let FOLLY_SAFE_DCHECK be erased by the optimizer in release
 builds

Summary:
[Folly] Let `FOLLY_SAFE_DCHECK` be erased by the optimizer in release builds.

Just like `DCHECK` is erased by the optimizer, not by the preprocessor, in release builds. `assert`, by contrast, is erased by the preprocessor. But the intention here is to mimic `DCHECK`.

This makes a difference if the expression uses a parameter or local variable. This way, the AST still sees a use of that parameter or local variable in release builds (the optimizer later removes that use). Rather than the AST not seeing any uses of that parameter or local variable, and consequently the compiler emitting a warning or error that the parameter or local variable is unused.

Even so, the expression is never evaluated in release builds. We ensure that by actually using `true || (expr)` as the conditional of our expanded expression in release builds. The `true` comes from `!folly::kIsDebug`, which is a `constexpr bool` expression and is `true` in release builds. The `||` short-circuits; the optimizer sees that the whole expanded expression statically evaluates to `static_cast<void>(0)` and removes it the expanded expression entirely.

Reviewed By: simpkins

Differential Revision: D3701227

fbshipit-source-id: e4fa48ee5a88e45dc08757c14d1944de734796ff
---
 folly/SafeAssert.cpp          |  7 +++++--
 folly/SafeAssert.h            | 16 +++++++---------
 folly/test/SafeAssertTest.cpp |  5 +++--
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/folly/SafeAssert.cpp b/folly/SafeAssert.cpp
index a8a64da2..3fc17039 100644
--- a/folly/SafeAssert.cpp
+++ b/folly/SafeAssert.cpp
@@ -22,15 +22,18 @@
 namespace folly { namespace detail {
 
 namespace {
+void writeStderr(const char* s, size_t len) {
+  writeFull(STDERR_FILENO, s, len);
+}
 void writeStderr(const char* s) {
-  writeFull(STDERR_FILENO, s, strlen(s));
+  writeStderr(s, strlen(s));
 }
 }  // namespace
 
 void assertionFailure(const char* expr, const char* msg, const char* file,
                       unsigned int line, const char* function) {
   writeStderr("\n\nAssertion failure: ");
-  writeStderr(expr);
+  writeStderr(expr + 1, strlen(expr) - 2);
   writeStderr("\nMessage: ");
   writeStderr(msg);
   writeStderr("\nFile: ");
diff --git a/folly/SafeAssert.h b/folly/SafeAssert.h
index 39d07423..73477a20 100644
--- a/folly/SafeAssert.h
+++ b/folly/SafeAssert.h
@@ -24,21 +24,19 @@
  * (containing msg) to stderr and abort()s. Just like CHECK(), but only
  * logs to stderr and only does async-signal-safe calls.
  */
-#define FOLLY_SAFE_CHECK(expr, msg) \
+#define FOLLY_SAFE_CHECK_IMPL(expr, expr_s, msg) \
   ((expr) ? static_cast<void>(0) : \
    ::folly::detail::assertionFailure( \
-       FB_STRINGIZE(expr), (msg), __FILE__, __LINE__, __PRETTY_FUNCTION__))
+       FB_STRINGIZE(expr_s), (msg), __FILE__, __LINE__, __PRETTY_FUNCTION__))
+#define FOLLY_SAFE_CHECK(expr, msg) FOLLY_SAFE_CHECK_IMPL((expr), (expr), (msg))
 
 /**
  * In debug mode, verify that the expression is true. Otherwise, do nothing
- * (do not even evaluate expr). Just like assert() or DCHECK(), but only
- * logs to stderr and only does async-signal-safe calls.
+ * (do not even evaluate expr). Just like DCHECK(), but only logs to stderr and
+ * only does async-signal-safe calls.
  */
-#ifdef NDEBUG
-#define FOLLY_SAFE_DCHECK(expr, msg) (static_cast<void>(0))
-#else
-#define FOLLY_SAFE_DCHECK FOLLY_SAFE_CHECK
-#endif
+#define FOLLY_SAFE_DCHECK(expr, msg) \
+  FOLLY_SAFE_CHECK_IMPL(!folly::kIsDebug || (expr), (expr), (msg))
 
 namespace folly { namespace detail {
 
diff --git a/folly/test/SafeAssertTest.cpp b/folly/test/SafeAssertTest.cpp
index 52069255..bfaa3dd6 100644
--- a/folly/test/SafeAssertTest.cpp
+++ b/folly/test/SafeAssertTest.cpp
@@ -24,7 +24,7 @@
 using namespace folly;
 
 void fail() {
-  FOLLY_SAFE_CHECK(0, "hello");
+  FOLLY_SAFE_CHECK(0 + 0, "hello");
 }
 
 void succeed() {
@@ -33,5 +33,6 @@ void succeed() {
 
 TEST(SafeAssert, AssertionFailure) {
   succeed();
-  EXPECT_DEATH(fail(), ".*Assertion failure:.*hello.*");
+  EXPECT_DEATH(fail(), "Assertion failure: 0 \\+ 0");
+  EXPECT_DEATH(fail(), "Message: hello");
 }
-- 
2.34.1