From: Jim Meyering <meyering@fb.com>
Date: Wed, 7 Jan 2015 21:45:19 +0000 (-0800)
Subject: folly: ProducerConsumerQueue: avoid many -Wsign-compare errors
X-Git-Tag: v0.22.0~7
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=7fdf8141e4d3737ac2c45c16830a8dbaac19b445;p=folly.git

folly: ProducerConsumerQueue: avoid many -Wsign-compare errors

Summary:
There were many templates instantiated with signed types.
Changing them to unsigned avoids the compilation failures
noted below.  However, we'll have to watch arc-diff-spawned
tests for new failures, just in case.

This is by far the largest undecomposable patch of those
I've written for this task so far.  It is possible that
some subset of these changes accomplishes the same goal,
but once I managed to get a successful compilation, I have
only re-reviewed it once and did not try to shrink it.

Test Plan:
Without these changes, we would get the following 32 errors from gcc-4.9:
https://phabricator.fb.com/P19692642
With them, (and all of the other patches attached to the parent task,
all of fbcode compiles and passes its tests (modulo preexisting
failures and aborts).

Reviewed By: delong.j@fb.com

Subscribers: trunkagent, folly-diffs@

FB internal diff: D1773461

Tasks: 5941250

Signature: t1:1773461:1420821964:61635f3a2efc95e8d4457c36e7a5ee3a0a50df23
---

diff --git a/folly/ProducerConsumerQueue.h b/folly/ProducerConsumerQueue.h
index ff1a4fe1..92eb0d4a 100644
--- a/folly/ProducerConsumerQueue.h
+++ b/folly/ProducerConsumerQueue.h
@@ -61,8 +61,8 @@ struct ProducerConsumerQueue : private boost::noncopyable {
     // (No real synchronization needed at destructor time: only one
     // thread can be doing this.)
     if (!boost::has_trivial_destructor<T>::value) {
-      int read = readIndex_;
-      int end = writeIndex_;
+      size_t read = readIndex_;
+      size_t end = writeIndex_;
       while (read != end) {
         records_[read].~T();
         if (++read == size_) {
@@ -168,8 +168,8 @@ private:
   const uint32_t size_;
   T* const records_;
 
-  std::atomic<int> readIndex_;
-  std::atomic<int> writeIndex_;
+  std::atomic<unsigned int> readIndex_;
+  std::atomic<unsigned int> writeIndex_;
 };
 
 }
diff --git a/folly/test/ProducerConsumerQueueBenchmark.cpp b/folly/test/ProducerConsumerQueueBenchmark.cpp
index 73f6a324..371f9f17 100644
--- a/folly/test/ProducerConsumerQueueBenchmark.cpp
+++ b/folly/test/ProducerConsumerQueueBenchmark.cpp
@@ -32,10 +32,10 @@ namespace {
 
 using namespace folly;
 
-typedef int ThroughputType;
+typedef unsigned int ThroughputType;
 typedef ProducerConsumerQueue<ThroughputType> ThroughputQueueType;
 
-typedef long LatencyType;
+typedef unsigned long LatencyType;
 typedef ProducerConsumerQueue<LatencyType> LatencyQueueType;
 
 template<class QueueType>
@@ -138,7 +138,7 @@ struct LatencyTest {
       pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset);
     }
     for (int i = 0; i < iters_; ++i) {
-      long enqueue_nsec;
+      unsigned long enqueue_nsec;
       while (!queue_.read(enqueue_nsec)) {
       }
 
diff --git a/folly/test/ProducerConsumerQueueTest.cpp b/folly/test/ProducerConsumerQueueTest.cpp
index de72c88b..b848690d 100644
--- a/folly/test/ProducerConsumerQueueTest.cpp
+++ b/folly/test/ProducerConsumerQueueTest.cpp
@@ -34,7 +34,7 @@ template<class T> struct TestTraits {
 };
 
 template<> struct TestTraits<std::string> {
-  int limit() const { return 1 << 22; }
+  unsigned int limit() const { return 1 << 22; }
   std::string generate() const { return std::string(12, ' '); }
 };
 
@@ -61,7 +61,10 @@ struct PerfTest {
   }
 
   void producer() {
-    for (int i = 0; i < traits_.limit(); ++i) {
+    // This is written differently than you might expect so that
+    // it does not run afoul of -Wsign-compare, regardless of the
+    // signedness of this loop's upper bound.
+    for (auto i = traits_.limit(); i > 0; --i) {
       while (!queue_.write(traits_.generate())) {
       }
     }
@@ -112,7 +115,7 @@ struct CorrectnessTest {
   {
     const size_t testSize = traits_.limit();
     testData_.reserve(testSize);
-    for (int i = 0; i < testSize; ++i) {
+    for (size_t i = 0; i < testSize; ++i) {
       testData_.push_back(traits_.generate());
     }
   }
@@ -200,13 +203,13 @@ void correctnessTestType(const std::string& type) {
 }
 
 struct DtorChecker {
-  static int numInstances;
+  static unsigned int numInstances;
   DtorChecker() { ++numInstances; }
   DtorChecker(const DtorChecker& o) { ++numInstances; }
   ~DtorChecker() { --numInstances; }
 };
 
-int DtorChecker::numInstances = 0;
+unsigned int DtorChecker::numInstances = 0;
 
 }