Replace some NDEBUG with kIsDebug (#1128)

Summary:
- Replace some calls sites from `NDEBUG` to `kIsDebug`.
- Not every call site is changed; there are a few useful cases of
  `NDEBUG` still. `xlog` macros are one of them. Another is where we
  conditionally define a variable in a local scope and then use it in an
  `assert`.
Pull Request resolved: https://github.com/facebook/folly/pull/1128

Reviewed By: aary

Differential Revision: D15490848

Pulled By: yfeldblum

fbshipit-source-id: 4f2d1689f90baa25d3ba9e99a2a388d9cfc43f18

CMakeLists.txt

@@ -789,6 +789,7 @@ if (BUILD_TESTS)
       TEST file_util_test HANGING
         SOURCES FileUtilTest.cpp
       TEST fingerprint_test SOURCES FingerprintTest.cpp
+      TEST fixed_string_test SOURCES FixedStringTest.cpp
       TEST format_other_test SOURCES FormatOtherTest.cpp
       TEST format_test SOURCES FormatTest.cpp
       TEST function_test BROKEN

folly/FixedString.h

@@ -79,6 +79,10 @@ constexpr std::size_t checkOverflowOrNpos(std::size_t i, std::size_t max) {
       : (i <= max ? i : (void(assertOutOfBounds()), max));
 }
 
+constexpr std::size_t checkOverflowIfDebug(std::size_t i, std::size_t size) {
+  return kIsDebug ? checkOverflow(i, size) : i;
+}
+
 // Intentionally NOT constexpr. See note above for assertOutOfBounds
 [[noreturn]] inline void assertNotNullTerminated() noexcept {
   assert(!"Non-null terminated string used to initialize a BasicFixedString");
@@ -1110,22 +1114,14 @@ class BasicFixedString : private detail::fixedstring::FixedStringBase {
    * \return `*(data() + i)`
    */
   constexpr Char& operator[](std::size_t i) noexcept {
-#ifdef NDEBUG
-    return data_[i];
-#else
-    return data_[detail::fixedstring::checkOverflow(i, size_)];
-#endif
+    return data_[detail::fixedstring::checkOverflowIfDebug(i, size_)];
   }
 
   /**
    * \overload
    */
   constexpr const Char& operator[](std::size_t i) const noexcept {
-#ifdef NDEBUG
-    return data_[i];
-#else
-    return data_[detail::fixedstring::checkOverflow(i, size_)];
-#endif
+    return data_[detail::fixedstring::checkOverflowIfDebug(i, size_)];
   }
 
   /**
@@ -1147,22 +1143,14 @@ class BasicFixedString : private detail::fixedstring::FixedStringBase {
    * \pre `!empty()`
    */
   constexpr Char& back() noexcept {
-#ifdef NDEBUG
-    return data_[size_ - 1u];
-#else
-    return data_[size_ - detail::fixedstring::checkOverflow(1u, size_)];
-#endif
+    return data_[size_ - detail::fixedstring::checkOverflowIfDebug(1u, size_)];
   }
 
   /**
    * \overload
    */
   constexpr const Char& back() const noexcept {
-#ifdef NDEBUG
-    return data_[size_ - 1u];
-#else
-    return data_[size_ - detail::fixedstring::checkOverflow(1u, size_)];
-#endif
+    return data_[size_ - detail::fixedstring::checkOverflowIfDebug(1u, size_)];
   }
 
   /**

folly/experimental/exception_tracer/StackTrace.cpp

@@ -61,46 +61,46 @@ bool StackTraceStack::pushCurrent() {
   }
   node->frameCount = n;
 
-  node->next = top_;
-  top_ = node;
+  node->next = state_[kTopIdx];
+  state_[kTopIdx] = node;
   return true;
 }
 
 bool StackTraceStack::pop() {
   checkGuard();
-  if (!top_) {
+  if (!state_[kTopIdx]) {
     return false;
   }
 
-  auto node = top_;
-  top_ = node->next;
+  auto node = state_[kTopIdx];
+  state_[kTopIdx] = node->next;
   node->deallocate();
   return true;
 }
 
 bool StackTraceStack::moveTopFrom(StackTraceStack& other) {
   checkGuard();
-  if (!other.top_) {
+  if (!other.state_[kTopIdx]) {
     return false;
   }
 
-  auto node = other.top_;
-  other.top_ = node->next;
-  node->next = top_;
-  top_ = node;
+  auto node = other.state_[kTopIdx];
+  other.state_[kTopIdx] = node->next;
+  node->next = state_[kTopIdx];
+  state_[kTopIdx] = node;
   return true;
 }
 
 void StackTraceStack::clear() {
   checkGuard();
-  while (top_) {
+  while (state_[kTopIdx]) {
     pop();
   }
 }
 
 StackTrace* StackTraceStack::top() {
   checkGuard();
-  return top_;
+  return state_[kTopIdx];
 }
 
 StackTrace* StackTraceStack::next(StackTrace* p) {

folly/experimental/exception_tracer/StackTrace.h

@@ -20,6 +20,8 @@
 #include <cstddef>
 #include <cstdint>
 
+#include <folly/Portability.h>
+
 namespace folly {
 namespace exception_tracer {
 
@@ -67,7 +69,7 @@ class StackTraceStack {
    * Is the stack empty?
    */
   bool empty() const {
-    return !top_;
+    return !state_[kTopIdx];
   }
 
   /**
@@ -82,21 +84,15 @@ class StackTraceStack {
   StackTrace* next(StackTrace* p);
 
  private:
+  static constexpr size_t kTopIdx = kIsDebug ? 1 : 0;
+
   // In debug mode, we assert that we're in zero-initialized memory by
-  // checking that the two guards around top_ are zero.
+  // checking that the two guards around the Node* from top() are zero.
   void checkGuard() const {
-#ifndef NDEBUG
-    assert(guard1_ == 0 && guard2_ == 0);
-#endif
+    assert(state_[0] == 0 && state_[2] == 0);
   }
 
-#ifndef NDEBUG
-  uintptr_t guard1_;
-#endif
-  Node* top_;
-#ifndef NDEBUG
-  uintptr_t guard2_;
-#endif
+  Node* state_[kIsDebug ? 3 : 1];
 };
 } // namespace exception_tracer
 } // namespace folly

folly/synchronization/LifoSem.h

@@ -26,6 +26,7 @@
 #include <folly/CachelinePadded.h>
 #include <folly/IndexedMemPool.h>
 #include <folly/Likely.h>
+#include <folly/Portability.h>
 #include <folly/Traits.h>
 #include <folly/lang/SafeAssert.h>
 #include <folly/synchronization/AtomicStruct.h>
@@ -185,9 +186,9 @@ struct LifoSemNode : public LifoSemRawNode<Atom> {
 
   void destroy() {
     handoff().~Handoff();
-#ifndef NDEBUG
-    memset(&this->raw, 'F', sizeof(this->raw));
-#endif
+    if (kIsDebug) {
+      memset(&this->raw, 'F', sizeof(this->raw));
+    }
   }
 
   Handoff& handoff() {