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

diff --git a/folly/SafeAssert.cpp b/folly/SafeAssert.cpp
index a8a64da246ef8c072a3307fd4ec6ef5e2ee889da..3fc170390dbda5146b9ebf506f815b1b8b9d67cb 100644 (file)
--- 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 39d07423dcb97f760c7d0a5a7dd2da8c67e030b6..73477a201c4812644e2ab54cde1d8a321838b1ca 100644 (file)
--- 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 52069255ca5c8c9ded6b58aa2c5c74ef87e78f51..bfaa3dd6a0da1ee93f421db345a6234acc2158f7 100644 (file)
--- 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");
 }