Bug 849666 - Make CheckedInt<T>::operator-() not depend on undefined behavior when negating minimum signed values, and add a test for this. Patch is something of a tag-team effort by bjacob and me. r=bjacob

This commit is contained in:
Jeff Walden 2013-03-11 18:45:22 -07:00
parent 03c7e434e8
commit b7bf374ed3
2 changed files with 31 additions and 20 deletions

View File

@ -40,6 +40,8 @@
namespace mozilla {
template<typename T> class CheckedInt;
namespace detail {
/*
@ -454,23 +456,32 @@ IsDivValid(T x, T y)
!(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 IsTSigned = IsSigned<T>::value>
struct OppositeIfSignedImpl
{
static T run(T x) { return -x; }
};
template<typename T, bool IsSigned = IsSigned<T>::value>
struct NegateImpl;
template<typename T>
struct OppositeIfSignedImpl<T, false>
struct NegateImpl<T, false>
{
static T run(T x) { return x; }
static CheckedInt<T> negate(const CheckedInt<T>& val)
{
// Handle negation separately for signed/unsigned, for simpler code and to
// avoid an MSVC warning negating an unsigned value.
return CheckedInt<T>(0, val.isValid() && val.mValue == 0);
}
};
template<typename T>
inline T
OppositeIfSigned(T x)
struct NegateImpl<T, true>
{
return OppositeIfSignedImpl<T>::run(x);
}
static CheckedInt<T> negate(const CheckedInt<T>& val)
{
// Watch out for the min-value, which (with twos-complement) can't be
// negated as -min-value is then (max-value + 1).
if (!val.isValid() || val.mValue == MinValue<T>::value)
return CheckedInt<T>(val.mValue, false);
return CheckedInt<T>(-val.mValue, true);
}
};
} // namespace detail
@ -560,6 +571,8 @@ class CheckedInt
"This type is not supported by CheckedInt");
}
friend class detail::NegateImpl<T>;
public:
/**
* Constructs a checked integer with given @a value. The checked integer is
@ -628,14 +641,7 @@ class CheckedInt
CheckedInt operator -() const
{
// Circumvent msvc warning about - applied to unsigned int.
// if we're unsigned, the only valid case anyway is 0
// in which case - is a no-op.
T result = detail::OppositeIfSigned(mValue);
/* Help the compiler perform RVO (return value optimization). */
return CheckedInt(result,
mIsValid && detail::IsSubValid(T(0),
mValue));
return detail::NegateImpl<T>::negate(*this);
}
/**

View File

@ -197,6 +197,8 @@ void test()
if (isTSigned) {
VERIFY_IS_VALID(-max);
VERIFY_IS_INVALID(-min);
VERIFY(-max - min == one);
VERIFY_IS_VALID(-max - one);
VERIFY_IS_VALID(negOne);
VERIFY_IS_VALID(-max + negOne);
@ -206,6 +208,9 @@ void test()
VERIFY_IS_VALID(negOne + negOne);
VERIFY(negOne + negOne == negTwo);
} else {
VERIFY_IS_INVALID(-max);
VERIFY_IS_VALID(-min);
VERIFY(min == zero);
VERIFY_IS_INVALID(negOne);
}