Bug 732875 - Further CheckedInt tweaks - r=Ms2ger,jwalden
authorBenoit Jacob <bjacob@mozilla.com>
Thu, 17 May 2012 10:07:24 -0400
changeset 94248 ae0b2ba1e47e
parent 94247 7b043e963658
child 94249 d6e2644caa5d
push id22704
push userryanvm@gmail.com
push date2012-05-18 03:26 +0000
Treeherderresults
reviewersMs2ger, jwalden
bugs732875
milestone15.0a1
Bug 732875 - Further CheckedInt tweaks - r=Ms2ger,jwalden
mfbt/CheckedInt.h
mfbt/tests/TestCheckedInt.cpp
--- a/mfbt/CheckedInt.h
+++ b/mfbt/CheckedInt.h
@@ -205,41 +205,40 @@ template<typename IntegerType>
 struct PositionOfSignBit
 {
     static const size_t value = CHAR_BIT * sizeof(IntegerType) - 1;
 };
 
 template<typename IntegerType>
 struct MinValue
 {
-    static IntegerType value()
-    {
-      // Bitwise ops may return a larger type, that's why we cast explicitly.
-      // In C++, left bit shifts on signed values is undefined by the standard
-      // unless the shifted value is representable.
-      // Notice that signed-to-unsigned conversions are always well-defined in
-      // the standard as the value congruent to 2**n, as expected. By contrast,
-      // unsigned-to-signed is only well-defined if the value is representable.
-      return IsSigned<IntegerType>::value
-             ? IntegerType(typename UnsignedType<IntegerType>::Type(1)
-                             << PositionOfSignBit<IntegerType>::value)
-             : IntegerType(0);
-    }
+  private:
+    typedef typename UnsignedType<IntegerType>::Type UnsignedIntegerType;
+    static const size_t PosOfSignBit = PositionOfSignBit<IntegerType>::value;
+
+  public:
+    // Bitwise ops may return a larger type, that's why we cast explicitly.
+    // In C++, left bit shifts on signed values is undefined by the standard
+    // unless the shifted value is representable.
+    // Notice that signed-to-unsigned conversions are always well-defined in
+    // the standard as the value congruent to 2**n, as expected. By contrast,
+    // unsigned-to-signed is only well-defined if the value is representable.
+    static const IntegerType value =
+        IsSigned<IntegerType>::value
+        ? IntegerType(UnsignedIntegerType(1) << PosOfSignBit)
+        : IntegerType(0);
 };
 
 template<typename IntegerType>
 struct MaxValue
 {
-    static IntegerType value()
-    {
-      // Tricksy, but covered by the unit test.
-      // Relies heavily on the return type of MinValue<IntegerType>::value()
-      // being IntegerType.
-      return ~MinValue<IntegerType>::value();
-    }
+    // Tricksy, but covered by the unit test.
+    // Relies heavily on the type of MinValue<IntegerType>::value
+    // being IntegerType.
+    static const IntegerType value = ~MinValue<IntegerType>::value;
 };
 
 /*
  * Step 3: Implement the actual validity checks.
  *
  * Ideas taken from IntegerLib, code different.
  */
 
@@ -273,49 +272,46 @@ template<typename T,
          bool IsUSigned = IsSigned<U>::value>
 struct IsInRangeImpl {};
 
 template<typename T, typename U>
 struct IsInRangeImpl<T, U, true, true>
 {
     static bool run(U x)
     {
-      return x <= MaxValue<T>::value() &&
-             x >= MinValue<T>::value();
+      return x <= MaxValue<T>::value && x >= MinValue<T>::value;
     }
 };
 
 template<typename T, typename U>
 struct IsInRangeImpl<T, U, false, false>
 {
     static bool run(U x)
     {
-      return x <= MaxValue<T>::value();
+      return x <= MaxValue<T>::value;
     }
 };
 
 template<typename T, typename U>
 struct IsInRangeImpl<T, U, true, false>
 {
     static bool run(U x)
     {
-      return sizeof(T) > sizeof(U)
-             ? true
-             : x <= U(MaxValue<T>::value());
+      return sizeof(T) > sizeof(U) || x <= U(MaxValue<T>::value);
     }
 };
 
 template<typename T, typename U>
 struct IsInRangeImpl<T, U, false, true>
 {
     static bool run(U x)
     {
       return sizeof(T) >= sizeof(U)
              ? x >= 0
-             : x >= 0 && x <= U(MaxValue<T>::value());
+             : x >= 0 && x <= U(MaxValue<T>::value);
     }
 };
 
 template<typename T, typename U>
 inline bool
 IsInRange(U x)
 {
   return IsInRangeImpl<T, U>::run(x);
@@ -361,18 +357,18 @@ struct IsMulValidImpl<T, IsSigned, true>
     }
 };
 
 template<typename T>
 struct IsMulValidImpl<T, true, false>
 {
     static bool run(T x, T y)
     {
-      const T max = MaxValue<T>::value();
-      const T min = MinValue<T>::value();
+      const T max = MaxValue<T>::value;
+      const T min = MinValue<T>::value;
 
       if (x == 0 || y == 0)
         return true;
 
       if (x > 0) {
         return y > 0
                ? x <= max / y
                : y >= min / x;
@@ -385,36 +381,34 @@ struct IsMulValidImpl<T, true, false>
     }
 };
 
 template<typename T>
 struct IsMulValidImpl<T, false, false>
 {
     static bool run(T x, T y)
     {
-      return y == 0 ||
-             x <= MaxValue<T>::value() / y;
+      return y == 0 ||  x <= MaxValue<T>::value / y;
     }
 };
 
 template<typename T>
 inline bool
 IsMulValid(T x, T y, T /* result not used */)
 {
   return IsMulValidImpl<T>::run(x, y);
 }
 
 template<typename T>
 inline bool
 IsDivValid(T x, T y)
 {
   // Keep in mind that in the signed case, min/-1 is invalid because abs(min)>max.
-  return IsSigned<T>::value
-         ? (y != 0) && (x != MinValue<T>::value() || y != T(-1))
-         : y != 0;
+  return y != 0 &&
+         !(IsSigned<T>::value && x == MinValue<T>::value && y == T(-1));
 }
 
 // This is just to shut up msvc warnings about negating unsigned ints.
 template<typename T, bool IsSigned = IsSigned<T>::value>
 struct OppositeIfSignedImpl
 {
     static T run(T x) { return -x; }
 };
--- a/mfbt/tests/TestCheckedInt.cpp
+++ b/mfbt/tests/TestCheckedInt.cpp
@@ -91,18 +91,18 @@ void test()
 
   testTwiceBiggerType<T>::run();
 
   typedef typename detail::UnsignedType<T>::Type unsignedT;
 
   VERIFY(sizeof(unsignedT) == sizeof(T));
   VERIFY(detail::IsSigned<unsignedT>::value == false);
 
-  const CheckedInt<T> max(detail::MaxValue<T>::value());
-  const CheckedInt<T> min(detail::MinValue<T>::value());
+  const CheckedInt<T> max(detail::MaxValue<T>::value);
+  const CheckedInt<T> min(detail::MinValue<T>::value);
 
   // Check min() and max(), since they are custom implementations and a mistake there
   // could potentially NOT be caught by any other tests... while making everything wrong!
 
   T bit = 1;
   for (size_t i = 0; i < sizeof(T) * CHAR_BIT - 1; i++)
   {
     VERIFY((min.value() & bit) == 0);
@@ -389,20 +389,20 @@ void test()
   { \
     bool isUSigned = detail::IsSigned<U>::value; \
     VERIFY_IS_VALID(CheckedInt<T>(U(0))); \
     VERIFY_IS_VALID(CheckedInt<T>(U(1))); \
     VERIFY_IS_VALID(CheckedInt<T>(U(100))); \
     if (isUSigned) \
       VERIFY_IS_VALID_IF(CheckedInt<T>(U(-1)), isTSigned); \
     if (sizeof(U) > sizeof(T)) \
-      VERIFY_IS_INVALID(CheckedInt<T>(U(detail::MaxValue<T>::value())+1)); \
-    VERIFY_IS_VALID_IF(CheckedInt<T>(detail::MaxValue<U>::value()), \
+      VERIFY_IS_INVALID(CheckedInt<T>(U(detail::MaxValue<T>::value) + 1)); \
+    VERIFY_IS_VALID_IF(CheckedInt<T>(detail::MaxValue<U>::value), \
       (sizeof(T) > sizeof(U) || ((sizeof(T) == sizeof(U)) && (isUSigned || !isTSigned)))); \
-    VERIFY_IS_VALID_IF(CheckedInt<T>(detail::MinValue<U>::value()), \
+    VERIFY_IS_VALID_IF(CheckedInt<T>(detail::MinValue<U>::value), \
       isUSigned == false ? 1 : \
       bool(isTSigned) == false ? 0 : \
       sizeof(T) >= sizeof(U)); \
   }
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(int8_t)
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(uint8_t)
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(int16_t)
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(uint16_t)