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
treeherdermozilla-central@e794cef56df6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMs2ger, jwalden
bugs732875
milestone15.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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)