Extend sign-compare warnings to gcc (take 2)

Remove `-Wno-sign-compare` option for GCC Suppress erroneous sign-compare warning in `c10::greater_than_max`(see https://godbolt.org/z/Tr3Msnz99) Fix sign-compare in torch/deploy, `caffe2::QTensor::dim32()` and `generate_proposals_op_test.cc` Pull Request resolved: pytorch#75544 Approved by: https://github.com/osalpekar
bowu1999 · Apr 13, 2022 · bdf5a87 · bdf5a87
1 parent 1601a4d
commit bdf5a87
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 8 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -787,7 +787,6 @@ if(NOT MSVC)
   string(APPEND CMAKE_CXX_FLAGS " -Wno-type-limits")
   string(APPEND CMAKE_CXX_FLAGS " -Wno-array-bounds")
   string(APPEND CMAKE_CXX_FLAGS " -Wno-unknown-pragmas")
-  string(APPEND CMAKE_CXX_FLAGS " -Wno-sign-compare")
   string(APPEND CMAKE_CXX_FLAGS " -Wno-unused-parameter")
   string(APPEND CMAKE_CXX_FLAGS " -Wno-unused-function")
   string(APPEND CMAKE_CXX_FLAGS " -Wno-unused-result")
@@ -798,8 +797,6 @@ if(NOT MSVC)
   if("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
     string(APPEND CMAKE_CXX_FLAGS " -Wno-range-loop-analysis")
     string(APPEND CMAKE_CXX_FLAGS " -Wno-pass-failed")
-    # sign-compare is not part of -Wall, see https://godbolt.org/z/s1YczM41T
-    string(APPEND CMAKE_CXX_FLAGS " -Wsign-compare")
   endif()
   if(CMAKE_COMPILER_IS_GNUCXX AND NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 7.0.0))
     string(APPEND CMAKE_CXX_FLAGS " -Wno-stringop-overflow")

diff --git a/c10/util/TypeSafeSignMath.h b/c10/util/TypeSafeSignMath.h
@@ -70,6 +70,14 @@ inline constexpr bool signs_differ(const T& a, const U& b) {
   return is_negative(a) != is_negative(b);
 }
 
+// Suppress sign compare warning when compiling with GCC
+// as later does not account for short-circuit rule before
+// raising the warning, see https://godbolt.org/z/Tr3Msnz99
+#ifdef __GNUC__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wsign-compare"
+#endif
+
 /// Returns true if x is greater than the greatest value of the type Limit
 template <typename Limit, typename T>
 inline constexpr bool greater_than_max(const T& x) {
@@ -78,6 +86,10 @@ inline constexpr bool greater_than_max(const T& x) {
   return can_overflow && x > std::numeric_limits<Limit>::max();
 }
 
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+
 /// Returns true if x < lowest(Limit). Standard comparison
 template <typename Limit, typename T>
 static inline constexpr bool less_than_lowest(

diff --git a/caffe2/core/qtensor.h b/caffe2/core/qtensor.h
@@ -187,7 +187,7 @@ class C10_EXPORT QTensor {
    * Returns the i-th dimension of the qtensor in int.
    */
   inline int dim32(const int i) const {
-    DCHECK_LT(i, dims_.size()) << "Exceeding ndim limit " << dims_.size();
+    DCHECK_LT(i, static_cast<int>(dims_.size())) << "Exceeding ndim limit " << dims_.size();
     DCHECK_GE(i, 0) << "Cannot have negative index";
     CAFFE_ENFORCE_LT(dims_[i], std::numeric_limits<int>::max());
     return static_cast<int>(dims_[i]);

diff --git a/caffe2/operators/generate_proposals_op_test.cc b/caffe2/operators/generate_proposals_op_test.cc
@@ -493,7 +493,7 @@ TEST(GenerateProposalsTest, TestRealDownSampledRotatedAngle0) {
       1.53593004e-01f,  -8.75087008e-02f, -4.92327996e-02f, -3.32239009e-02f};
 
   // Add angle in bbox deltas
-  int num_boxes = scores.size();
+  auto num_boxes = scores.size();
   CHECK_EQ(bbx.size() / 4, num_boxes);
   vector<float> bbx_with_angle(num_boxes * box_dim);
   // bbx (deltas) is in shape (A * 4, H, W). Insert angle delta
@@ -666,7 +666,7 @@ TEST(GenerateProposalsTest, TestRealDownSampledRotated) {
       1.53593004e-01f,  -8.75087008e-02f, -4.92327996e-02f, -3.32239009e-02f};
 
   // Add angle in bbox deltas
-  int num_boxes = scores.size();
+  auto num_boxes = scores.size();
   CHECK_EQ(bbx.size() / 4, num_boxes);
   vector<float> bbx_with_angle(num_boxes * box_dim);
   // bbx (deltas) is in shape (A * 4, H, W). Insert angle delta

diff --git a/torch/csrc/deploy/deploy.cpp b/torch/csrc/deploy/deploy.cpp
@@ -301,8 +301,7 @@ int LoadBalancer::acquire() {
   size_t minusers = SIZE_MAX;
   int minIdx = 0;
   for (size_t i = 0; i < n_; ++i, ++last) {
-    // NOLINTNEXTLINE(clang-diagnostic-sign-compare)
-    if (last >= n_) {
+    if (last >= static_cast<int>(n_)) {
       last = 0;
     }
     uint64_t prev = 0;