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 90d7d84f4d69529631f84fd9d83480b005692b44..af2235e3d74b142a7382d04f07a46e4f8d6c51d6 100644 (file)
--- 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 <class Tgt>
+typename std::enable_if<
+    !std::is_same<Tgt, bool>::value && std::is_integral<Tgt>::value,
+    Expected<Tgt, ConversionCode>>::type
+convertTo(const bool& value) noexcept {
+  return static_cast<Tgt>(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 71918c4e34ab067759aaa1a8514b5247492e7f0e..23fe52b29562bbec27385a3a60279bbe9836e299 100644 (file)
--- 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 20634a7a796ce4be25d01c64c5df433a6db4713c..94763881e8d2e36be0230322177137c2bd327ef7 100644 (file)
--- 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 a78f6e05b9f2e9f310685e30cdc61efbd4de924e..999a7a257d5238f721d1322ea5bc45b5f030c1f8 100644 (file)
--- a/folly/Traits.h
+++ b/folly/Traits.h
@@ -423,11 +423,13 @@ struct is_negative_impl<T, false> {
 // 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 <typename RHS, RHS rhs, typename LHS>
 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 79dfe74e4d80b56fbeaf883481e81a5cf6a4361d..83ead386c5712aba00c28695ce6b9c619d26c190 100644 (file)
--- 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 <bool HAYSTACK_ALIGNED>
 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 be0cbf60c19c43927042109f791f2ea339423fb9..99a622836a9a6a9ead3fd1a84aa44a7a80f3945b 100644 (file)
--- a/folly/detail/ThreadLocalDetail.cpp
+++ b/folly/detail/ThreadLocalDetail.cpp
@@ -34,7 +34,7 @@ void StaticMetaBase::onThreadExit(void* ptr) {
 #else
   std::unique_ptr<ThreadEntry> threadEntry(static_cast<ThreadEntry*>(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 bf648b56ca1545dabb0f42594c2fa094079b2fc7..7abc002f1e32218a058e153cc196c21777622700 100644 (file)
--- 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 f8368942f0393dc955e10931f4a1fe100b66a181..e9ce723f8259f2b1e6465117fc055dcb72df9a1c 100644 (file)
--- 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 078438d9c10a2cf4cb33eded66d4691778be5bb0..c170669f05b9ba79b2dd50395ee82645ecaa447f 100644 (file)
--- a/folly/experimental/bser/Dump.cpp
+++ b/folly/experimental/bser/Dump.cpp
@@ -211,15 +211,15 @@ std::unique_ptr<folly::IOBuf> toBserIOBuf(folly::dynamic const& dyn,
   auto magicptr = hdrbuf + sizeof(kMagic);
   auto lenptr = hdrbuf + hdrlen;
 
-  if (len > std::numeric_limits<int32_t>::max()) {
+  if (len > uint64_t(std::numeric_limits<int32_t>::max())) {
     *magicptr = (int8_t)BserType::Int64;
     *(int64_t*)lenptr = (int64_t)len;
     hdrlen += sizeof(int64_t);
-  } else if (len > std::numeric_limits<int16_t>::max()) {
+  } else if (len > uint64_t(std::numeric_limits<int16_t>::max())) {
     *magicptr = (int8_t)BserType::Int32;
     *(int32_t*)lenptr = (int32_t)len;
     hdrlen += sizeof(int32_t);
-  } else if (len > std::numeric_limits<int8_t>::max()) {
+  } else if (len > uint64_t(std::numeric_limits<int8_t>::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 5b0945a049e6135fbfbcf9843fad032518442653..33ecab3a9d83a5fb1d59dca169484c68dd99c325 100644 (file)
--- 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 570f629f28c98fb3f8cdb57865dc80d43ac55a0d..de4124f77d14447a1a32fbf124b397d031dc4ab0 100644 (file)
--- 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<intptr_t>(stackLimit) % sizeof(uint64_t));
-  CHECK_EQ(0, stackSize % sizeof(uint64_t));
+  CHECK_EQ(reinterpret_cast<intptr_t>(stackLimit) % sizeof(uint64_t), 0u);
+  CHECK_EQ(stackSize % sizeof(uint64_t), 0u);
   uint64_t* begin = reinterpret_cast<uint64_t*>(stackLimit);
   uint64_t* end = reinterpret_cast<uint64_t*>(stackLimit + stackSize);
 
@@ -82,8 +82,8 @@ void Fiber::init(bool recordStackUsed) {
   recordStackUsed_ = recordStackUsed;
   if (UNLIKELY(recordStackUsed_ && !stackFilledWithMagic_)) {
     CHECK_EQ(
-        0, reinterpret_cast<intptr_t>(fiberStackLimit_) % sizeof(uint64_t));
-    CHECK_EQ(0, fiberStackSize_ % sizeof(uint64_t));
+        reinterpret_cast<intptr_t>(fiberStackLimit_) % sizeof(uint64_t), 0u);
+    CHECK_EQ(fiberStackSize_ % sizeof(uint64_t), 0u);
     std::fill(
         reinterpret_cast<uint64_t*>(fiberStackLimit_),
         reinterpret_cast<uint64_t*>(fiberStackLimit_ + fiberStackSize_),

diff --git a/folly/fibers/FiberManagerMap.cpp b/folly/fibers/FiberManagerMap.cpp
index d96fb82e4aa4196027f26de4c213391cee63cc50..021a5676ea6fa56750535667bf0bcaa4fb0b80aa 100644 (file)
--- a/folly/fibers/FiberManagerMap.cpp
+++ b/folly/fibers/FiberManagerMap.cpp
@@ -76,7 +76,7 @@ class GlobalCache {
   std::unique_ptr<FiberManager> eraseImpl(EventBaseT& evb) {
     std::lock_guard<std::mutex> 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 5b8e1b3d7382a1e14a44fd9d71efa46096b6b1af..2bd5a883b291423402d597008574e8ed8215558b 100644 (file)
--- 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 bd2bab0412d4293c833fcdf1edc02721f74c8e0e..500b3b54db10b4778552f9cab403ecac640aaa20 100644 (file)
--- 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 98e7d744b0ea4d73ba34bf3c30c00e2dc98c4b11..66c7cb168688d45b923f0e629742672b3620391a 100644 (file)
--- 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<uintptr_t>(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<uintptr_t>(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 5af387507ee153e732a3d3dffbd8db6c8ef045f7..84317bf72de19f64295ac48f8e599a910dd773b0 100755 (executable)
--- 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 1a1dceb88d3f62093f13df95a98796721a8e4ec9..17c2b12e960628844974b0e64484823596fd8b19 100755 (executable)
--- 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 3e8c853d66c6b1799a0ec1e5ce2fd943ccea5753..02694a536b034bccebb2cf8f39fa95a72caca798 100644 (file)
--- a/folly/stats/MultiLevelTimeSeries-defs.h
+++ b/folly/stats/MultiLevelTimeSeries-defs.h
@@ -27,7 +27,7 @@ MultiLevelTimeSeries<VT, CT>::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<VT, CT>::MultiLevelTimeSeries(
     size_t nBuckets,
     std::initializer_list<Duration> 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 c8f0cdf2d7805887a656993b798a06e156a88edb..c1c082edccf300ef40d8a90b13b4f86d08d78774 100644 (file)
--- 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];
   }