From fd01e2bfa6a9ca0cfe3f3317ccc66173c683c769 Mon Sep 17 00:00:00 2001 From: Giuseppe Ottaviano Date: Mon, 12 Sep 2016 14:00:40 -0700 Subject: [PATCH] Do not mess with NDEBUG in fbstring Summary: Temporarily overriding `NDEBUG` can have unexpected side-effects depending on the implementation of ``. Better use an explicit macro for `assert()` that we can explicitly disable. Reviewed By: Gownta Differential Revision: D3850717 fbshipit-source-id: b1e7fce89ab4120b0224e9c6bf668d983b3d2d31 --- folly/FBString.h | 175 ++++++++++++++++++++++------------------------- 1 file changed, 82 insertions(+), 93 deletions(-) diff --git a/folly/FBString.h b/folly/FBString.h index 7a9d6ede..c7698103 100644 --- a/folly/FBString.h +++ b/folly/FBString.h @@ -31,14 +31,11 @@ #pragma GCC system_header -// When used as std::string replacement always disable assertions. -#ifndef NDEBUG -#define NDEBUG -#define FOLLY_DEFINED_NDEBUG_FOR_FBSTRING -#endif // NDEBUG - #include "basic_fbstring_malloc.h" +// When used as std::string replacement always disable assertions. +#define FBSTRING_ASSERT(expr) /* empty */ + #else // !_LIBSTDCXX_FBSTRING #include @@ -65,6 +62,9 @@ #endif #endif +// When used in folly, assertions are not disabled. +#define FBSTRING_ASSERT(expr) assert(expr) + #endif // We defined these here rather than including Likely.h to avoid @@ -134,7 +134,7 @@ inline std::pair copy_n( template inline void podFill(Pod* b, Pod* e, T c) { - assert(b && e && b <= e); + FBSTRING_ASSERT(b && e && b <= e); /*static*/ if (sizeof(T) == 1) { memset(b, c, e - b); } else { @@ -166,8 +166,8 @@ inline void podFill(Pod* b, Pod* e, T c) { */ template inline void podCopy(const Pod* b, const Pod* e, Pod* d) { - assert(e >= b); - assert(d >= e || d + (e - b) <= b); + FBSTRING_ASSERT(e >= b); + FBSTRING_ASSERT(d >= e || d + (e - b) <= b); memcpy(d, b, (e - b) * sizeof(Pod)); } @@ -177,7 +177,7 @@ inline void podCopy(const Pod* b, const Pod* e, Pod* d) { */ template inline void podMove(const Pod* b, const Pod* e, Pod* d) { - assert(e >= b); + FBSTRING_ASSERT(e >= b); memmove(d, b, (e - b) * sizeof(*b)); } @@ -321,7 +321,7 @@ public: fbstring_core() noexcept { reset(); } fbstring_core(const fbstring_core & rhs) { - assert(&rhs != this); + FBSTRING_ASSERT(&rhs != this); switch (rhs.category()) { case Category::isSmall: copySmall(rhs); @@ -335,8 +335,8 @@ public: default: fbstring_detail::assume_unreachable(); } - assert(size() == rhs.size()); - assert(memcmp(data(), rhs.data(), size() * sizeof(Char)) == 0); + FBSTRING_ASSERT(size() == rhs.size()); + FBSTRING_ASSERT(memcmp(data(), rhs.data(), size() * sizeof(Char)) == 0); } fbstring_core(fbstring_core&& goner) noexcept { @@ -356,12 +356,9 @@ public: } else { initLarge(data, size); } -#ifndef NDEBUG -#ifndef _LIBSTDCXX_FBSTRING - assert(this->size() == size); - assert(size == 0 || memcmp(this->data(), data, size * sizeof(Char)) == 0); -#endif -#endif + FBSTRING_ASSERT(this->size() == size); + FBSTRING_ASSERT( + size == 0 || memcmp(this->data(), data, size * sizeof(Char)) == 0); } ~fbstring_core() noexcept { @@ -388,8 +385,8 @@ public: const size_t allocatedSize, AcquireMallocatedString) { if (size > 0) { - assert(allocatedSize >= size + 1); - assert(data[size] == '\0'); + FBSTRING_ASSERT(allocatedSize >= size + 1); + FBSTRING_ASSERT(data[size] == '\0'); // Use the medium string storage ml_.data_ = data; ml_.size_ = size; @@ -432,11 +429,11 @@ public: const Char * c_str() const { auto const c = category(); if (c == Category::isSmall) { - assert(small_[smallSize()] == '\0'); + FBSTRING_ASSERT(small_[smallSize()] == '\0'); return small_; } - assert(c == Category::isMedium || c == Category::isLarge); - assert(ml_.data_[ml_.size_] == '\0'); + FBSTRING_ASSERT(c == Category::isMedium || c == Category::isLarge); + FBSTRING_ASSERT(ml_.data_[ml_.size_] == '\0'); return ml_.data_; } @@ -465,7 +462,7 @@ public: default: fbstring_detail::assume_unreachable(); } - assert(capacity() >= minCapacity); + FBSTRING_ASSERT(capacity() >= minCapacity); } Char* expandNoinit( @@ -512,7 +509,7 @@ private: ? maxSmallSize << (8 * (sizeof(size_t) - sizeof(Char))) : maxSmallSize << 2; small_[0] = '\0'; - assert(category() == Category::isSmall && size() == 0); + FBSTRING_ASSERT(category() == Category::isSmall && size() == 0); } struct RefCounted { @@ -537,7 +534,7 @@ private: static void decrementRefs(Char * p) { auto const dis = fromData(p); size_t oldcnt = dis->refCount_.fetch_sub(1, std::memory_order_acq_rel); - assert(oldcnt > 0); + FBSTRING_ASSERT(oldcnt > 0); if (oldcnt == 1) { free(dis); } @@ -566,9 +563,9 @@ private: const size_t currentSize, const size_t currentCapacity, const size_t newCapacity) { - assert(newCapacity > 0 && newCapacity > currentSize); + FBSTRING_ASSERT(newCapacity > 0 && newCapacity > currentSize); auto const dis = fromData(data); - assert(dis->refCount_.load(std::memory_order_acquire) == 1); + FBSTRING_ASSERT(dis->refCount_.load(std::memory_order_acquire) == 1); // Don't forget to allocate one extra Char for the terminating // null. In this case, however, one Char is already part of the // struct. @@ -577,7 +574,7 @@ private: sizeof(RefCounted) + currentSize * sizeof(Char), sizeof(RefCounted) + currentCapacity * sizeof(Char), sizeof(RefCounted) + newCapacity * sizeof(Char))); - assert(result->refCount_.load(std::memory_order_acquire) == 1); + FBSTRING_ASSERT(result->refCount_.load(std::memory_order_acquire) == 1); return result; } }; @@ -639,10 +636,10 @@ private: "Corrupt memory layout for fbstring."); size_t smallSize() const { - assert(category() == Category::isSmall); + FBSTRING_ASSERT(category() == Category::isSmall); constexpr auto shift = kIsLittleEndian ? 0 : 2; auto smallShifted = static_cast(small_[maxSmallSize]) >> shift; - assert(static_cast(maxSmallSize) >= smallShifted); + FBSTRING_ASSERT(static_cast(maxSmallSize) >= smallShifted); return static_cast(maxSmallSize) - smallShifted; } @@ -650,11 +647,11 @@ private: // Warning: this should work with uninitialized strings too, // so don't assume anything about the previous value of // small_[maxSmallSize]. - assert(s <= maxSmallSize); + FBSTRING_ASSERT(s <= maxSmallSize); constexpr auto shift = kIsLittleEndian ? 0 : 2; small_[maxSmallSize] = (maxSmallSize - s) << shift; small_[s] = '\0'; - assert(category() == Category::isSmall && size() == s); + FBSTRING_ASSERT(category() == Category::isSmall && size() == s); } void copySmall(const fbstring_core&); @@ -691,7 +688,8 @@ inline void fbstring_core::copySmall(const fbstring_core& rhs) { // which stores a short string's length, is shared with the // ml_.capacity field). ml_ = rhs.ml_; - assert(category() == Category::isSmall && this->size() == rhs.size()); + FBSTRING_ASSERT( + category() == Category::isSmall && this->size() == rhs.size()); } template @@ -705,7 +703,7 @@ inline void fbstring_core::copyMedium(const fbstring_core& rhs) { rhs.ml_.data_, rhs.ml_.data_ + rhs.ml_.size_ + 1, ml_.data_); ml_.size_ = rhs.ml_.size_; ml_.setCapacity(allocSize / sizeof(Char) - 1, Category::isMedium); - assert(category() == Category::isMedium); + FBSTRING_ASSERT(category() == Category::isMedium); } template @@ -713,7 +711,7 @@ inline void fbstring_core::copyLarge(const fbstring_core& rhs) { // Large strings are just refcounted ml_ = rhs.ml_; RefCounted::incrementRefs(ml_.data_); - assert(category() == Category::isLarge && size() == rhs.size()); + FBSTRING_ASSERT(category() == Category::isLarge && size() == rhs.size()); } // Small strings are bitblitted @@ -787,14 +785,14 @@ inline void fbstring_core::initLarge( template inline Char* fbstring_core::mutableDataLarge() { - assert(category() == Category::isLarge); + FBSTRING_ASSERT(category() == Category::isLarge); if (RefCounted::refs(ml_.data_) > 1) { // Ensure unique. size_t effectiveCapacity = ml_.capacity(); auto const newRC = RefCounted::create(&effectiveCapacity); // If this fails, someone placed the wrong capacity in an // fbstring. - assert(effectiveCapacity >= ml_.capacity()); + FBSTRING_ASSERT(effectiveCapacity >= ml_.capacity()); // Also copies terminator. fbstring_detail::podCopy( ml_.data_, ml_.data_ + ml_.size_ + 1, newRC->data_); @@ -806,7 +804,7 @@ inline Char* fbstring_core::mutableDataLarge() { template inline void fbstring_core::reserveLarge(size_t minCapacity) { - assert(category() == Category::isLarge); + FBSTRING_ASSERT(category() == Category::isLarge); // Ensure unique if (RefCounted::refs(ml_.data_) > 1) { // We must make it unique regardless; in-place reallocation is @@ -832,13 +830,13 @@ inline void fbstring_core::reserveLarge(size_t minCapacity) { ml_.data_ = newRC->data_; ml_.setCapacity(minCapacity, Category::isLarge); } - assert(capacity() >= minCapacity); + FBSTRING_ASSERT(capacity() >= minCapacity); } } template inline void fbstring_core::reserveMedium(const size_t minCapacity) { - assert(category() == Category::isMedium); + FBSTRING_ASSERT(category() == Category::isMedium); // String is not shared if (minCapacity <= ml_.capacity()) { return; // nothing to do, there's enough room @@ -864,14 +862,14 @@ inline void fbstring_core::reserveMedium(const size_t minCapacity) { fbstring_detail::podCopy( ml_.data_, ml_.data_ + ml_.size_ + 1, nascent.ml_.data_); nascent.swap(*this); - assert(capacity() >= minCapacity); + FBSTRING_ASSERT(capacity() >= minCapacity); } } template inline void fbstring_core::reserveSmall( size_t minCapacity, const bool disableSSO) { - assert(category() == Category::isSmall); + FBSTRING_ASSERT(category() == Category::isSmall); if (!disableSSO && minCapacity <= maxSmallSize) { // small // Nothing to do, everything stays put @@ -896,7 +894,7 @@ inline void fbstring_core::reserveSmall( ml_.data_ = newRC->data_; ml_.size_ = size; ml_.setCapacity(minCapacity, Category::isLarge); - assert(capacity() >= minCapacity); + FBSTRING_ASSERT(capacity() >= minCapacity); } } @@ -906,7 +904,7 @@ inline Char* fbstring_core::expandNoinit( bool expGrowth, /* = false */ bool disableSSO /* = FBSTRING_DISABLE_SSO */) { // Strategy is simple: make room, then change size - assert(capacity() >= size()); + FBSTRING_ASSERT(capacity() >= size()); size_t sz, newSz; if (category() == Category::isSmall) { sz = smallSize(); @@ -925,19 +923,20 @@ inline Char* fbstring_core::expandNoinit( reserve(expGrowth ? std::max(newSz, 1 + capacity() * 3 / 2) : newSz); } } - assert(capacity() >= newSz); + FBSTRING_ASSERT(capacity() >= newSz); // Category can't be small - we took care of that above - assert(category() == Category::isMedium || category() == Category::isLarge); + FBSTRING_ASSERT( + category() == Category::isMedium || category() == Category::isLarge); ml_.size_ = newSz; ml_.data_[newSz] = '\0'; - assert(size() == newSz); + FBSTRING_ASSERT(size() == newSz); return ml_.data_ + sz; } template inline void fbstring_core::shrinkSmall(const size_t delta) { // Check for underflow - assert(delta <= smallSize()); + FBSTRING_ASSERT(delta <= smallSize()); setSmallSize(smallSize() - delta); } @@ -945,14 +944,14 @@ template inline void fbstring_core::shrinkMedium(const size_t delta) { // Medium strings and unique large strings need no special // handling. - assert(ml_.size_ >= delta); + FBSTRING_ASSERT(ml_.size_ >= delta); ml_.size_ -= delta; ml_.data_[ml_.size_] = '\0'; } template inline void fbstring_core::shrinkLarge(const size_t delta) { - assert(ml_.size_ >= delta); + FBSTRING_ASSERT(ml_.size_ >= delta); // Shared large string, must make unique. This is because of the // durn terminator must be written, which may trample the shared // data. @@ -988,7 +987,7 @@ public: return const_cast(backend_.data()); } void shrink(size_t delta) { - assert(delta <= size()); + FBSTRING_ASSERT(delta <= size()); backend_.resize(size() - delta); } Char* expandNoinit(size_t delta) { @@ -1031,7 +1030,6 @@ template > #endif class basic_fbstring { - static void enforce( bool condition, void (*throw_exc)(const char*), @@ -1052,25 +1050,20 @@ class basic_fbstring { begin()[size()] == '\0'; } - struct Invariant; - friend struct Invariant; struct Invariant { Invariant& operator=(const Invariant&) = delete; -#ifndef NDEBUG - explicit Invariant(const basic_fbstring& s) : s_(s) { - assert(s_.isSane()); + explicit Invariant(const basic_fbstring& s) noexcept : s_(s) { + FBSTRING_ASSERT(s_.isSane()); } - ~Invariant() { - assert(s_.isSane()); + ~Invariant() noexcept { + FBSTRING_ASSERT(s_.isSane()); } - private: + + private: const basic_fbstring& s_; -#else - explicit Invariant(const basic_fbstring&) {} -#endif }; -public: + public: // types typedef T traits_type; typedef typename traits_type::char_type value_type; @@ -1273,18 +1266,18 @@ public: // C++11 21.4.5, element access: const value_type& front() const { return *begin(); } const value_type& back() const { - assert(!empty()); + FBSTRING_ASSERT(!empty()); // Should be begin()[size() - 1], but that branches twice return *(end() - 1); } value_type& front() { return *begin(); } value_type& back() { - assert(!empty()); + FBSTRING_ASSERT(!empty()); // Should be begin()[size() - 1], but that branches twice return *(end() - 1); } void pop_back() { - assert(!empty()); + FBSTRING_ASSERT(!empty()); store_.shrink(1); } @@ -1879,7 +1872,7 @@ inline void basic_fbstring::resize( auto pData = store_.expandNoinit(delta); fbstring_detail::podFill(pData, pData + delta, c); } - assert(this->size() == n); + FBSTRING_ASSERT(this->size() == n); } template @@ -1889,7 +1882,7 @@ inline basic_fbstring& basic_fbstring::append( auto desiredSize = size() + str.size(); #endif append(str.data(), str.size()); - assert(size() == desiredSize); + FBSTRING_ASSERT(size() == desiredSize); return *this; } @@ -1923,7 +1916,7 @@ inline basic_fbstring& basic_fbstring::append( // info. std::less_equal le; if (FBSTRING_UNLIKELY(le(oldData, s) && !le(oldData + oldSize, s))) { - assert(le(s + n, oldData + oldSize)); + FBSTRING_ASSERT(le(s + n, oldData + oldSize)); // expandNoinit() could have moved the storage, restore the source. s = data() + (s - oldData); fbstring_detail::podMove(s, s + n, pData); @@ -1931,7 +1924,7 @@ inline basic_fbstring& basic_fbstring::append( fbstring_detail::podCopy(s, s + n, pData); } - assert(size() == oldSize + n); + FBSTRING_ASSERT(size() == oldSize + n); return *this; } @@ -1965,7 +1958,7 @@ inline basic_fbstring& basic_fbstring::assign( // s can alias this, we need to use podMove. fbstring_detail::podMove(s, s + n, store_.mutableData()); store_.shrink(size() - n); - assert(size() == n); + FBSTRING_ASSERT(size() == n); } else { // If n is larger than size(), s cannot alias this string's // storage. @@ -1975,7 +1968,7 @@ inline basic_fbstring& basic_fbstring::assign( fbstring_detail::podCopy(s, s + n, store_.expandNoinit(n)); } - assert(size() == n); + FBSTRING_ASSERT(size() == n); return *this; } @@ -2003,8 +1996,8 @@ basic_fbstring::getlineImpl(istream_type & is, value_type delim) { break; } - assert(size == this->size()); - assert(size == capacity()); + FBSTRING_ASSERT(size == this->size()); + FBSTRING_ASSERT(size == capacity()); // Start at minimum allocation 63 + terminator = 64. reserve(std::max(63, 3 * size / 2)); // Clear the error so we can continue reading. @@ -2054,7 +2047,7 @@ basic_fbstring::find( // Here we know that the last char matches // Continue in pedestrian mode for (size_t j = 0;;) { - assert(j < nsize); + FBSTRING_ASSERT(j < nsize); if (i[j] != needle[j]) { // Not found, we can skip // Compute the skip value lazily @@ -2083,7 +2076,7 @@ basic_fbstring::insertImplDiscr( const_iterator i, size_type n, value_type c, std::true_type) { Invariant checker(*this); - assert(i >= cbegin() && i <= cend()); + FBSTRING_ASSERT(i >= cbegin() && i <= cend()); const size_type pos = i - cbegin(); auto oldSize = size(); @@ -2114,10 +2107,10 @@ basic_fbstring::insertImpl( std::forward_iterator_tag) { Invariant checker(*this); - assert(i >= cbegin() && i <= cend()); + FBSTRING_ASSERT(i >= cbegin() && i <= cend()); const size_type pos = i - cbegin(); auto n = std::distance(s1, s2); - assert(n >= 0); + FBSTRING_ASSERT(n >= 0); auto oldSize = size(); store_.expandNoinit(n, /* expGrowth = */ true); @@ -2153,9 +2146,9 @@ inline basic_fbstring& basic_fbstring::replaceImplDiscr( const value_type* s, size_type n, std::integral_constant) { - assert(i1 <= i2); - assert(begin() <= i1 && i1 <= end()); - assert(begin() <= i2 && i2 <= end()); + FBSTRING_ASSERT(i1 <= i2); + FBSTRING_ASSERT(begin() <= i1 && i1 <= end()); + FBSTRING_ASSERT(begin() <= i2 && i2 <= end()); return replace(i1, i2, s, s + n); } @@ -2174,7 +2167,7 @@ inline basic_fbstring& basic_fbstring::replaceImplDiscr( std::fill(i1, i2, c); insert(i2, n2 - n1, c); } - assert(isSane()); + FBSTRING_ASSERT(isSane()); return *this; } @@ -2228,9 +2221,9 @@ inline void basic_fbstring::replaceImpl( } auto const n1 = i2 - i1; - assert(n1 >= 0); + FBSTRING_ASSERT(n1 >= 0); auto const n2 = std::distance(s1, s2); - assert(n2 >= 0); + FBSTRING_ASSERT(n2 >= 0); if (n1 > n2) { // shrinks @@ -2241,7 +2234,7 @@ inline void basic_fbstring::replaceImpl( s1 = fbstring_detail::copy_n(s1, n1, i1).first; insert(i2, s1, s2); } - assert(isSane()); + FBSTRING_ASSERT(isSane()); } template @@ -2805,8 +2798,4 @@ FOLLY_FBSTRING_HASH #undef throw #undef FBSTRING_LIKELY #undef FBSTRING_UNLIKELY - -#ifdef FOLLY_DEFINED_NDEBUG_FOR_FBSTRING -#undef NDEBUG -#undef FOLLY_DEFINED_NDEBUG_FOR_FBSTRING -#endif // FOLLY_DEFINED_NDEBUG_FOR_FBSTRING +#undef FBSTRING_ASSERT -- 2.34.1