Bug 732875 - Further CheckedInt tweaks - r=Ms2ger,jwalden
--- 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)