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 ff1a4fe13b3dbe35903c4adf5dee3ac0a85f10d4..92eb0d4a96f74b02561e5e501044aa1e1981946a 100644 (file)
--- 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 73f6a3246a94f189b9b6c7a63fd17759fcb1315e..371f9f179709ebf732e5211697035d4b25be09a5 100644 (file)
--- 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 de72c88bbb0a9f84b56ecab2dd22b0c1014f24d1..b848690daff180c767c650b0d84491194efa35f8 100644 (file)
--- 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;
 
 }