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

diff --git a/folly/Traits.h b/folly/Traits.h
index f09a9136cab7a2a82f1bb59d1c8bb5703cb9c75b..e5f6b38cfcc024a42fd168858fd4f1e2a1e0af3e 100644 (file)
--- 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 7a80c6cc7041d17cff799c0641be0d5b4221c64e..784524e13822080466eb6d5a78315f98ce8bc18d 100644 (file)
--- 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) {