Bug 732875 - Further CheckedInt tweaks - r=Ms2ger,jwalden
authorBenoit Jacob <bjacob@mozilla.com>
Thu, 17 May 2012 10:07:24 -0400
changeset 94244 ae0b2ba1e47e56000d684d4ac72b35a13c08b19e
parent 94243 7b043e9636582671cd1815f103ffa86804a7c807
child 94245 d6e2644caa5dbd4286e8de80e106fe8f1d80a417
push id9538
push userbjacob@mozilla.com
push dateThu, 17 May 2012 19:12:47 +0000
treeherdermozilla-inbound@d6e2644caa5d [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)