From 423eed2d591e7b43c0aeb4e3814ebd5b1a127aa9 Mon Sep 17 00:00:00 2001
From: Louis Brandy <ldbrandy@fb.com>
Date: Wed, 19 Jun 2013 17:01:38 -0700
Subject: [PATCH] fix tautological comparisons in Conv.h

Summary:
We have an off-by-one in our enable_if/sfinae logic here. We do not want to actually do the comparison in `less_than` when the rhs is exactly the minimum possible lhs. This results in a tautological comparison.

I added a unit test for these traits that test all the various sfinae cases.

Test Plan: See the clang tautological warnings go away. Under gcc, rerun all tests. All pass.

Reviewed By: marcelo.juchem@fb.com

FB internal diff: D856869
---
 folly/Traits.h            |  4 ++--
 folly/test/TraitsTest.cpp | 15 +++++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/folly/Traits.h b/folly/Traits.h
index f09a9136..e5f6b38c 100644
--- a/folly/Traits.h
+++ b/folly/Traits.h
@@ -332,7 +332,7 @@ template <typename RHS, RHS rhs, typename LHS>
 bool less_than_impl(
   typename std::enable_if<
     (rhs <= std::numeric_limits<LHS>::max()
-      && rhs >= std::numeric_limits<LHS>::min()),
+      && rhs > std::numeric_limits<LHS>::min()),
     LHS
   >::type const lhs
 ) {
@@ -352,7 +352,7 @@ bool less_than_impl(
 template <typename RHS, RHS rhs, typename LHS>
 bool less_than_impl(
   typename std::enable_if<
-    (rhs < std::numeric_limits<LHS>::min()),
+    (rhs <= std::numeric_limits<LHS>::min()),
     LHS
   >::type const
 ) {
diff --git a/folly/test/TraitsTest.cpp b/folly/test/TraitsTest.cpp
index 7a80c6cc..784524e1 100644
--- a/folly/test/TraitsTest.cpp
+++ b/folly/test/TraitsTest.cpp
@@ -96,6 +96,21 @@ TEST(Traits, is_negative) {
   EXPECT_FALSE(folly::is_non_positive(1u));
 }
 
+TEST(Traits, relational) {
+  // We test, especially, the edge cases to make sure we don't
+  // trip -Wtautological-comparisons
+
+  EXPECT_FALSE((folly::less_than<uint8_t, 0u,   uint8_t>(0u)));
+  EXPECT_FALSE((folly::less_than<uint8_t, 0u,   uint8_t>(254u)));
+  EXPECT_FALSE((folly::less_than<uint8_t, 255u, uint8_t>(255u)));
+  EXPECT_TRUE( (folly::less_than<uint8_t, 255u, uint8_t>(254u)));
+
+  EXPECT_FALSE((folly::greater_than<uint8_t, 0u,   uint8_t>(0u)));
+  EXPECT_TRUE( (folly::greater_than<uint8_t, 0u,   uint8_t>(254u)));
+  EXPECT_FALSE((folly::greater_than<uint8_t, 255u, uint8_t>(255u)));
+  EXPECT_FALSE((folly::greater_than<uint8_t, 255u, uint8_t>(254u)));
+}
+
 struct CompleteType {};
 struct IncompleteType;
 TEST(Traits, is_complete) {
-- 
2.34.1