From: Christopher Dykes Date: Fri, 2 Dec 2016 22:23:36 +0000 (-0800) Subject: Begin making folly compile cleanly with a few of MSVC's sign mismatch warnings enabled X-Git-Tag: v2016.12.05.00~2 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=047e9e39c43e16a9c2221406f85fa3d8221c1f35;p=folly.git Begin making folly compile cleanly with a few of MSVC's sign mismatch warnings enabled Summary: This makes the changes required to allow folly to compile cleanly with the sign/unsigned mismatch warnings 4388 and 4389, as well as with warnings 4804 and 4805, which are about comparisons between scalars and bool. Most of the changes in this are to `DCHECK_*` calls which are translated to a templated call which does the comparison internally based on the inferred type of the argument, which for a literal `0` is `int`, causing the warnings to get generated when the comparison is done. Reviewed By: yfeldblum Differential Revision: D4253427 fbshipit-source-id: cd17973a78e948a62c886a2959f9abf40a69f9f5 --- diff --git a/folly/Conv.h b/folly/Conv.h index 90d7d84f..af2235e3 100644 --- a/folly/Conv.h +++ b/folly/Conv.h @@ -1175,6 +1175,19 @@ parseTo(StringPiece src, Tgt& out) { namespace detail { +/** + * Bool to integral doesn't need any special checks, and this + * overload means we aren't trying to see if a bool is less than + * an integer. + */ +template +typename std::enable_if< + !std::is_same::value && std::is_integral::value, + Expected>::type +convertTo(const bool& value) noexcept { + return static_cast(value ? 1 : 0); +} + /** * Checked conversion from integral to integral. The checks are only * performed when meaningful, e.g. conversion from int to long goes diff --git a/folly/MemoryMapping.cpp b/folly/MemoryMapping.cpp index 71918c4e..23fe52b2 100644 --- a/folly/MemoryMapping.cpp +++ b/folly/MemoryMapping.cpp @@ -309,7 +309,7 @@ MemoryMapping::~MemoryMapping() { void MemoryMapping::advise(int advice) const { advise(advice, 0, mapLength_); } void MemoryMapping::advise(int advice, size_t offset, size_t length) const { - CHECK_LE(offset + length, mapLength_) + CHECK_LE(offset + length, size_t(mapLength_)) << " offset: " << offset << " length: " << length << " mapLength_: " << mapLength_; diff --git a/folly/String.cpp b/folly/String.cpp index 20634a7a..94763881 100644 --- a/folly/String.cpp +++ b/folly/String.cpp @@ -540,7 +540,7 @@ size_t hexDumpLine(const void* ptr, size_t offset, size_t size, } line.append(16 - n, ' '); line.push_back('|'); - DCHECK_EQ(line.size(), 78); + DCHECK_EQ(line.size(), 78u); return n; } diff --git a/folly/Traits.h b/folly/Traits.h index a78f6e05..999a7a25 100644 --- a/folly/Traits.h +++ b/folly/Traits.h @@ -423,11 +423,13 @@ struct is_negative_impl { // inside what are really static ifs (not executed because of the templated // types) that violate -Wsign-compare and/or -Wbool-compare so suppress them // in order to not prevent all calling code from using it. -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wsign-compare" +FOLLY_PUSH_WARNING +FOLLY_GCC_DISABLE_WARNING(sign-compare) #if __GNUC_PREREQ(5, 0) -#pragma GCC diagnostic ignored "-Wbool-compare" +FOLLY_GCC_DISABLE_WARNING(bool-compare) #endif +FOLLY_MSVC_DISABLE_WARNING(4388) // sign-compare +FOLLY_MSVC_DISABLE_WARNING(4804) // bool-compare template bool less_than_impl(LHS const lhs) { @@ -445,7 +447,7 @@ bool greater_than_impl(LHS const lhs) { lhs > rhs; } -#pragma GCC diagnostic pop +FOLLY_POP_WARNING } // namespace detail { diff --git a/folly/detail/RangeSse42.cpp b/folly/detail/RangeSse42.cpp index 79dfe74e..83ead386 100644 --- a/folly/detail/RangeSse42.cpp +++ b/folly/detail/RangeSse42.cpp @@ -88,9 +88,9 @@ static size_t qfind_first_byte_of_needles16(const StringPieceLite haystack, // helper method for case where needles.size() <= 16 size_t qfind_first_byte_of_needles16(const StringPieceLite haystack, const StringPieceLite needles) { - DCHECK_GT(haystack.size(), 0); - DCHECK_GT(needles.size(), 0); - DCHECK_LE(needles.size(), 16); + DCHECK_GT(haystack.size(), 0u); + DCHECK_GT(needles.size(), 0u); + DCHECK_LE(needles.size(), 16u); if ((needles.size() <= 2 && haystack.size() >= 256) || // must bail if we can't even SSE-load a single segment of haystack (haystack.size() < 16 && @@ -142,7 +142,7 @@ template size_t scanHaystackBlock(const StringPieceLite haystack, const StringPieceLite needles, uint64_t blockStartIdx) { - DCHECK_GT(needles.size(), 16); // should handled by *needles16() method + DCHECK_GT(needles.size(), 16u); // should handled by *needles16() method DCHECK(blockStartIdx + 16 <= haystack.size() || (page_for(haystack.data() + blockStartIdx) == page_for(haystack.data() + blockStartIdx + 15))); diff --git a/folly/detail/ThreadLocalDetail.cpp b/folly/detail/ThreadLocalDetail.cpp index be0cbf60..99a62283 100644 --- a/folly/detail/ThreadLocalDetail.cpp +++ b/folly/detail/ThreadLocalDetail.cpp @@ -34,7 +34,7 @@ void StaticMetaBase::onThreadExit(void* ptr) { #else std::unique_ptr threadEntry(static_cast(ptr)); #endif - DCHECK_GT(threadEntry->elementsCapacity, 0); + DCHECK_GT(threadEntry->elementsCapacity, 0u); auto& meta = *threadEntry->meta; // Make sure this ThreadEntry is available if ThreadLocal A is accessed in diff --git a/folly/experimental/DynamicParser.cpp b/folly/experimental/DynamicParser.cpp index bf648b56..7abc002f 100644 --- a/folly/experimental/DynamicParser.cpp +++ b/folly/experimental/DynamicParser.cpp @@ -108,7 +108,7 @@ void DynamicParser::ParserStack::Pop::operator()() noexcept { stackPtr_->value_ = value_; if (stackPtr_->unmaterializedSubErrorKeys_.empty()) { // There should be the current error, and the root. - CHECK_GE(stackPtr_->subErrors_.size(), 2) + CHECK_GE(stackPtr_->subErrors_.size(), 2u) << "Internal bug: out of suberrors"; stackPtr_->subErrors_.pop_back(); } else { diff --git a/folly/experimental/Instructions.h b/folly/experimental/Instructions.h index f8368942..e9ce723f 100644 --- a/folly/experimental/Instructions.h +++ b/folly/experimental/Instructions.h @@ -44,11 +44,11 @@ struct Default { return __builtin_popcountll(value); } static FOLLY_ALWAYS_INLINE int ctz(uint64_t value) { - DCHECK_GT(value, 0); + DCHECK_GT(value, 0u); return __builtin_ctzll(value); } static FOLLY_ALWAYS_INLINE int clz(uint64_t value) { - DCHECK_GT(value, 0); + DCHECK_GT(value, 0u); return __builtin_clzll(value); } static FOLLY_ALWAYS_INLINE uint64_t blsr(uint64_t value) { diff --git a/folly/experimental/bser/Dump.cpp b/folly/experimental/bser/Dump.cpp index 078438d9..c170669f 100644 --- a/folly/experimental/bser/Dump.cpp +++ b/folly/experimental/bser/Dump.cpp @@ -211,15 +211,15 @@ std::unique_ptr toBserIOBuf(folly::dynamic const& dyn, auto magicptr = hdrbuf + sizeof(kMagic); auto lenptr = hdrbuf + hdrlen; - if (len > std::numeric_limits::max()) { + if (len > uint64_t(std::numeric_limits::max())) { *magicptr = (int8_t)BserType::Int64; *(int64_t*)lenptr = (int64_t)len; hdrlen += sizeof(int64_t); - } else if (len > std::numeric_limits::max()) { + } else if (len > uint64_t(std::numeric_limits::max())) { *magicptr = (int8_t)BserType::Int32; *(int32_t*)lenptr = (int32_t)len; hdrlen += sizeof(int32_t); - } else if (len > std::numeric_limits::max()) { + } else if (len > uint64_t(std::numeric_limits::max())) { *magicptr = (int8_t)BserType::Int16; *(int16_t*)lenptr = (int16_t)len; hdrlen += sizeof(int16_t); diff --git a/folly/experimental/observer/detail/GraphCycleDetector.h b/folly/experimental/observer/detail/GraphCycleDetector.h index 5b0945a0..33ecab3a 100644 --- a/folly/experimental/observer/detail/GraphCycleDetector.h +++ b/folly/experimental/observer/detail/GraphCycleDetector.h @@ -40,7 +40,7 @@ class GraphCycleDetector { } auto& nodes = edges_[from]; - DCHECK_EQ(0, nodes.count(to)); + DCHECK_EQ(nodes.count(to), 0u); nodes.insert(to); return true; diff --git a/folly/fibers/Fiber.cpp b/folly/fibers/Fiber.cpp index 570f629f..de4124f7 100644 --- a/folly/fibers/Fiber.cpp +++ b/folly/fibers/Fiber.cpp @@ -38,8 +38,8 @@ std::thread::id localThreadId() { /* Size of the region from p + nBytes down to the last non-magic value */ static size_t nonMagicInBytes(unsigned char* stackLimit, size_t stackSize) { - CHECK_EQ(0, reinterpret_cast(stackLimit) % sizeof(uint64_t)); - CHECK_EQ(0, stackSize % sizeof(uint64_t)); + CHECK_EQ(reinterpret_cast(stackLimit) % sizeof(uint64_t), 0u); + CHECK_EQ(stackSize % sizeof(uint64_t), 0u); uint64_t* begin = reinterpret_cast(stackLimit); uint64_t* end = reinterpret_cast(stackLimit + stackSize); @@ -82,8 +82,8 @@ void Fiber::init(bool recordStackUsed) { recordStackUsed_ = recordStackUsed; if (UNLIKELY(recordStackUsed_ && !stackFilledWithMagic_)) { CHECK_EQ( - 0, reinterpret_cast(fiberStackLimit_) % sizeof(uint64_t)); - CHECK_EQ(0, fiberStackSize_ % sizeof(uint64_t)); + reinterpret_cast(fiberStackLimit_) % sizeof(uint64_t), 0u); + CHECK_EQ(fiberStackSize_ % sizeof(uint64_t), 0u); std::fill( reinterpret_cast(fiberStackLimit_), reinterpret_cast(fiberStackLimit_ + fiberStackSize_), diff --git a/folly/fibers/FiberManagerMap.cpp b/folly/fibers/FiberManagerMap.cpp index d96fb82e..021a5676 100644 --- a/folly/fibers/FiberManagerMap.cpp +++ b/folly/fibers/FiberManagerMap.cpp @@ -76,7 +76,7 @@ class GlobalCache { std::unique_ptr eraseImpl(EventBaseT& evb) { std::lock_guard lg(mutex_); - DCHECK_EQ(1, map_.count(&evb)); + DCHECK_EQ(map_.count(&evb), 1u); auto ret = std::move(map_[&evb]); map_.erase(&evb); diff --git a/folly/futures/Barrier.cpp b/folly/futures/Barrier.cpp index 5b8e1b3d..2bd5a883 100644 --- a/folly/futures/Barrier.cpp +++ b/folly/futures/Barrier.cpp @@ -25,7 +25,7 @@ Barrier::Barrier(uint32_t n) Barrier::~Barrier() { auto block = controlBlock_.load(std::memory_order_relaxed); auto prev = block->valueAndReaderCount.load(std::memory_order_relaxed); - DCHECK_EQ(prev >> kReaderShift, 0); + DCHECK_EQ(prev >> kReaderShift, 0u); auto val = prev & kValueMask; auto p = promises(block); diff --git a/folly/io/IOBuf.cpp b/folly/io/IOBuf.cpp index bd2bab04..500b3b54 100644 --- a/folly/io/IOBuf.cpp +++ b/folly/io/IOBuf.cpp @@ -995,7 +995,7 @@ bool IOBufEqual::operator()(const IOBuf& a, const IOBuf& b) const { return false; } size_t n = std::min(ba.size(), bb.size()); - DCHECK_GT(n, 0); + DCHECK_GT(n, 0u); if (memcmp(ba.data(), bb.data(), n)) { return false; } diff --git a/folly/io/IOBuf.h b/folly/io/IOBuf.h index 98e7d744..66c7cb16 100644 --- a/folly/io/IOBuf.h +++ b/folly/io/IOBuf.h @@ -1331,8 +1331,8 @@ class IOBuf { static inline uintptr_t packFlagsAndSharedInfo(uintptr_t flags, SharedInfo* info) { uintptr_t uinfo = reinterpret_cast(info); - DCHECK_EQ(flags & ~kFlagMask, 0); - DCHECK_EQ(uinfo & kFlagMask, 0); + DCHECK_EQ(flags & ~kFlagMask, 0u); + DCHECK_EQ(uinfo & kFlagMask, 0u); return flags | uinfo; } @@ -1342,7 +1342,7 @@ class IOBuf { inline void setSharedInfo(SharedInfo* info) { uintptr_t uinfo = reinterpret_cast(info); - DCHECK_EQ(uinfo & kFlagMask, 0); + DCHECK_EQ(uinfo & kFlagMask, 0u); flagsAndSharedInfo_ = (flagsAndSharedInfo_ & kFlagMask) | uinfo; } @@ -1352,12 +1352,12 @@ class IOBuf { // flags_ are changed from const methods inline void setFlags(uintptr_t flags) const { - DCHECK_EQ(flags & ~kFlagMask, 0); + DCHECK_EQ(flags & ~kFlagMask, 0u); flagsAndSharedInfo_ |= flags; } inline void clearFlags(uintptr_t flags) const { - DCHECK_EQ(flags & ~kFlagMask, 0); + DCHECK_EQ(flags & ~kFlagMask, 0u); flagsAndSharedInfo_ &= ~flags; } diff --git a/folly/portability/Sockets.cpp b/folly/portability/Sockets.cpp index 5af38750..84317bf7 100755 --- a/folly/portability/Sockets.cpp +++ b/folly/portability/Sockets.cpp @@ -337,7 +337,7 @@ ssize_t sendmsg(int s, const struct msghdr* message, int fl) { (int)message->msg_iov[i].iov_len, message->msg_flags); } - if (r == -1 || r != message->msg_iov[i].iov_len) { + if (r == -1 || size_t(r) != message->msg_iov[i].iov_len) { errno = translate_wsa_error(WSAGetLastError()); if (WSAGetLastError() == WSAEWOULDBLOCK && bytesSent > 0) { return bytesSent; diff --git a/folly/portability/SysUio.cpp b/folly/portability/SysUio.cpp index 1a1dceb8..17c2b12e 100755 --- a/folly/portability/SysUio.cpp +++ b/folly/portability/SysUio.cpp @@ -95,7 +95,7 @@ static ssize_t doVecOperation(int fd, const iovec* iov, int count) { return -1; } - if (res == curLen) { + if (size_t(res) == curLen) { curIov++; if (curIov < count) { curBase = iov[curIov].iov_base; diff --git a/folly/stats/MultiLevelTimeSeries-defs.h b/folly/stats/MultiLevelTimeSeries-defs.h index 3e8c853d..02694a53 100644 --- a/folly/stats/MultiLevelTimeSeries-defs.h +++ b/folly/stats/MultiLevelTimeSeries-defs.h @@ -27,7 +27,7 @@ MultiLevelTimeSeries::MultiLevelTimeSeries( size_t nLevels, const Duration levelDurations[]) : cachedTime_(), cachedSum_(0), cachedCount_(0) { - CHECK_GT(nLevels, 0); + CHECK_GT(nLevels, 0u); CHECK(levelDurations); levels_.reserve(nLevels); @@ -46,10 +46,10 @@ MultiLevelTimeSeries::MultiLevelTimeSeries( size_t nBuckets, std::initializer_list durations) : cachedTime_(), cachedSum_(0), cachedCount_(0) { - CHECK_GT(durations.size(), 0); + CHECK_GT(durations.size(), 0u); levels_.reserve(durations.size()); - int i = 0; + size_t i = 0; Duration prev{0}; for (auto dur : durations) { if (dur == Duration(0)) { diff --git a/folly/stats/MultiLevelTimeSeries.h b/folly/stats/MultiLevelTimeSeries.h index c8f0cdf2..c1c082ed 100644 --- a/folly/stats/MultiLevelTimeSeries.h +++ b/folly/stats/MultiLevelTimeSeries.h @@ -100,7 +100,7 @@ class MultiLevelTimeSeries { */ const Level& getLevel(int level) const { CHECK(level >= 0); - CHECK_LT(level, levels_.size()); + CHECK_LT(size_t(level), levels_.size()); return levels_[level]; }