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

@@ -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: ");

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 {
 

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");
 }