Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(758)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: base/numerics/safe_math_impl.h

Issue 2480953002: Make Checked* functions better align with GCC/Clang intrinsics (Closed)
Patch Set: nits + compile fix Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/numerics/safe_math.h ('k') | base/numerics/safe_numerics_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/numerics/safe_math_impl.h
diff --git a/base/numerics/safe_math_impl.h b/base/numerics/safe_math_impl.h
index ca3bd5f6ab403f7b7b7e7918f65d0f3a2b46aa1f..1886d6080c92189c4963a6f2d14cf56c34c8214b 100644
--- a/base/numerics/safe_math_impl.h
+++ b/base/numerics/safe_math_impl.h
@@ -95,7 +95,7 @@ struct PositionOfSignBit {
// This is used for UnsignedAbs, where we need to support floating-point
// template instantiations even though we don't actually support the operations.
-// However, there is no corresponding implementation of e.g. CheckedUnsignedAbs,
+// However, there is no corresponding implementation of e.g. SafeUnsignedAbs,
// so the float versions will not compile.
template <typename Numeric,
bool IsInteger = std::numeric_limits<Numeric>::is_integer,
@@ -132,43 +132,40 @@ constexpr T BinaryComplement(T x) {
// way to coalesce things into the CheckedNumericState specializations below.
template <typename T>
-typename std::enable_if<std::numeric_limits<T>::is_integer, T>::type
-CheckedAdd(T x, T y, bool* validity) {
+typename std::enable_if<std::numeric_limits<T>::is_integer, bool>::type
+CheckedAdd(T x, T y, T* result) {
// Since the value of x+y is undefined if we have a signed type, we compute
// it using the unsigned type of the same size.
typedef typename UnsignedIntegerForSize<T>::type UnsignedDst;
UnsignedDst ux = static_cast<UnsignedDst>(x);
UnsignedDst uy = static_cast<UnsignedDst>(y);
UnsignedDst uresult = static_cast<UnsignedDst>(ux + uy);
+ *result = static_cast<T>(uresult);
// Addition is valid if the sign of (x + y) is equal to either that of x or
// that of y.
- if (std::numeric_limits<T>::is_signed) {
- *validity = HasSignBit(BinaryComplement(
- static_cast<UnsignedDst>((uresult ^ ux) & (uresult ^ uy))));
- } else { // Unsigned is either valid or overflow.
- *validity = BinaryComplement(x) >= y;
- }
- return static_cast<T>(uresult);
+ return (std::numeric_limits<T>::is_signed)
+ ? HasSignBit(BinaryComplement(
+ static_cast<UnsignedDst>((uresult ^ ux) & (uresult ^ uy))))
+ : (BinaryComplement(x) >=
+ y); // Unsigned is either valid or underflow.
}
template <typename T>
-typename std::enable_if<std::numeric_limits<T>::is_integer, T>::type
-CheckedSub(T x, T y, bool* validity) {
+typename std::enable_if<std::numeric_limits<T>::is_integer, bool>::type
+CheckedSub(T x, T y, T* result) {
// Since the value of x+y is undefined if we have a signed type, we compute
// it using the unsigned type of the same size.
typedef typename UnsignedIntegerForSize<T>::type UnsignedDst;
UnsignedDst ux = static_cast<UnsignedDst>(x);
UnsignedDst uy = static_cast<UnsignedDst>(y);
UnsignedDst uresult = static_cast<UnsignedDst>(ux - uy);
+ *result = static_cast<T>(uresult);
// Subtraction is valid if either x and y have same sign, or (x-y) and x have
// the same sign.
- if (std::numeric_limits<T>::is_signed) {
- *validity = HasSignBit(
- BinaryComplement(static_cast<UnsignedDst>((uresult ^ ux) & (ux ^ uy))));
- } else { // Unsigned is either valid or underflow.
- *validity = x >= y;
- }
- return static_cast<T>(uresult);
+ return (std::numeric_limits<T>::is_signed)
+ ? HasSignBit(BinaryComplement(
+ static_cast<UnsignedDst>((uresult ^ ux) & (ux ^ uy))))
+ : (x >= y);
}
// Integer multiplication is a bit complicated. In the fast case we just
@@ -178,135 +175,143 @@ CheckedSub(T x, T y, bool* validity) {
template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
sizeof(T) * 2 <= sizeof(uintmax_t),
- T>::type
-CheckedMul(T x, T y, bool* validity) {
+ bool>::type
+CheckedMul(T x, T y, T* result) {
typedef typename TwiceWiderInteger<T>::type IntermediateType;
IntermediateType tmp =
static_cast<IntermediateType>(x) * static_cast<IntermediateType>(y);
- *validity = DstRangeRelationToSrcRange<T>(tmp) == RANGE_VALID;
- return static_cast<T>(tmp);
+ *result = static_cast<T>(tmp);
+ return DstRangeRelationToSrcRange<T>(tmp) == RANGE_VALID;
}
template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
std::numeric_limits<T>::is_signed &&
(sizeof(T) * 2 > sizeof(uintmax_t)),
- T>::type
-CheckedMul(T x, T y, bool* validity) {
- // If either side is zero then the result will be zero.
- if (!x || !y) {
- *validity = true;
- return static_cast<T>(0);
- }
- if (x > 0) {
- if (y > 0) {
- *validity = x <= std::numeric_limits<T>::max() / y;
+ bool>::type
+CheckedMul(T x, T y, T* result) {
+ if (x && y) {
+ if (x > 0) {
+ if (y > 0) {
+ if (x > std::numeric_limits<T>::max() / y)
+ return false;
+ } else {
+ if (y < std::numeric_limits<T>::min() / x)
+ return false;
+ }
} else {
- *validity = y >= std::numeric_limits<T>::min() / x;
- }
- } else {
- if (y > 0) {
- *validity = x >= std::numeric_limits<T>::min() / y;
- } else {
- *validity = y >= std::numeric_limits<T>::max() / x;
+ if (y > 0) {
+ if (x < std::numeric_limits<T>::min() / y)
+ return false;
+ } else {
+ if (y < std::numeric_limits<T>::max() / x)
+ return false;
+ }
}
}
- return static_cast<T>(*validity ? x * y : 0);
+ *result = x * y;
+ return true;
}
template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
!std::numeric_limits<T>::is_signed &&
(sizeof(T) * 2 > sizeof(uintmax_t)),
- T>::type
-CheckedMul(T x, T y, bool* validity) {
- *validity = (y == 0 || x <= std::numeric_limits<T>::max() / y);
- return static_cast<T>(*validity ? x * y : 0);
+ bool>::type
+CheckedMul(T x, T y, T* result) {
+ *result = x * y;
+ return (y == 0 || x <= std::numeric_limits<T>::max() / y);
}
// Division just requires a check for a zero denominator or an invalid negation
// on signed min/-1.
template <typename T>
-T CheckedDiv(T x,
- T y,
- bool* validity,
- typename std::enable_if<std::numeric_limits<T>::is_integer,
- int>::type = 0) {
- if ((y == 0) ||
- (std::numeric_limits<T>::is_signed &&
- x == std::numeric_limits<T>::min() && y == static_cast<T>(-1))) {
- *validity = false;
- return static_cast<T>(0);
+typename std::enable_if<std::numeric_limits<T>::is_integer, bool>::type
+CheckedDiv(T x, T y, T* result) {
+ if (y && (!std::numeric_limits<T>::is_signed ||
+ x != std::numeric_limits<T>::min() || y != static_cast<T>(-1))) {
+ *result = x / y;
+ return true;
}
-
- *validity = true;
- return static_cast<T>(x / y);
+ return false;
}
template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
std::numeric_limits<T>::is_signed,
- T>::type
-CheckedMod(T x, T y, bool* validity) {
- *validity = y > 0;
- return static_cast<T>(*validity ? x % y : 0);
+ bool>::type
+CheckedMod(T x, T y, T* result) {
+ if (y > 0) {
+ *result = static_cast<T>(x % y);
+ return true;
+ }
+ return false;
}
template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
!std::numeric_limits<T>::is_signed,
- T>::type
-CheckedMod(T x, T y, bool* validity) {
- *validity = y != 0;
- return static_cast<T>(*validity ? x % y : 0);
+ bool>::type
+CheckedMod(T x, T y, T* result) {
+ if (y != 0) {
+ *result = static_cast<T>(x % y);
+ return true;
+ }
+ return false;
}
template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
std::numeric_limits<T>::is_signed,
- T>::type
-CheckedNeg(T value, bool* validity) {
- *validity = value != std::numeric_limits<T>::min();
+ bool>::type
+CheckedNeg(T value, T* result) {
// The negation of signed min is min, so catch that one.
- return static_cast<T>(*validity ? -value : 0);
+ if (value != std::numeric_limits<T>::min()) {
+ *result = static_cast<T>(-value);
+ return true;
+ }
+ return false;
}
template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
!std::numeric_limits<T>::is_signed,
- T>::type
-CheckedNeg(T value, bool* validity) {
- // The only legal unsigned negation is zero.
- *validity = !value;
- return static_cast<T>(
- *validity ? -static_cast<typename SignedIntegerForSize<T>::type>(value)
- : 0);
+ bool>::type
+CheckedNeg(T value, T* result) {
+ if (!value) { // The only legal unsigned negation is zero.
+ *result = static_cast<T>(0);
+ return true;
+ }
+ return false;
}
template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
std::numeric_limits<T>::is_signed,
- T>::type
-CheckedAbs(T value, bool* validity) {
- *validity = value != std::numeric_limits<T>::min();
- return static_cast<T>(*validity ? std::abs(value) : 0);
+ bool>::type
+CheckedAbs(T value, T* result) {
+ if (value != std::numeric_limits<T>::min()) {
+ *result = std::abs(value);
+ return true;
+ }
+ return false;
}
template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
!std::numeric_limits<T>::is_signed,
- T>::type
-CheckedAbs(T value, bool* validity) {
+ bool>::type
+CheckedAbs(T value, T* result) {
// T is unsigned, so |value| must already be positive.
- *validity = true;
- return value;
+ *result = value;
+ return true;
}
template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
std::numeric_limits<T>::is_signed,
typename UnsignedIntegerForSize<T>::type>::type
-CheckedUnsignedAbs(T value) {
+SafeUnsignedAbs(T value) {
typedef typename UnsignedIntegerForSize<T>::type UnsignedT;
return value == std::numeric_limits<T>::min()
? static_cast<UnsignedT>(std::numeric_limits<T>::max()) + 1
@@ -317,19 +322,34 @@ template <typename T>
typename std::enable_if<std::numeric_limits<T>::is_integer &&
!std::numeric_limits<T>::is_signed,
T>::type
-CheckedUnsignedAbs(T value) {
+SafeUnsignedAbs(T value) {
// T is unsigned, so |value| must already be positive.
return static_cast<T>(value);
}
-// These are the floating point stubs that the compiler needs to see. Only the
-// negation operation is ever called.
-#define BASE_FLOAT_ARITHMETIC_STUBS(NAME) \
- template <typename T> \
- typename std::enable_if<std::numeric_limits<T>::is_iec559, T>::type \
- Checked##NAME(T, T, bool*) { \
- NOTREACHED(); \
- return static_cast<T>(0); \
+template <typename T>
+typename std::enable_if<std::numeric_limits<T>::is_iec559, T>::type CheckedNeg(
+ T value,
+ bool*) {
+ NOTREACHED();
+ return static_cast<T>(-value);
+}
+
+template <typename T>
+typename std::enable_if<std::numeric_limits<T>::is_iec559, T>::type CheckedAbs(
+ T value,
+ bool*) {
+ NOTREACHED();
+ return static_cast<T>(std::abs(value));
+}
+
+// These are the floating point stubs that the compiler needs to see.
+#define BASE_FLOAT_ARITHMETIC_STUBS(NAME) \
+ template <typename T> \
+ typename std::enable_if<std::numeric_limits<T>::is_iec559, bool>::type \
+ Checked##NAME(T, T, T*) { \
+ NOTREACHED(); \
+ return static_cast<T>(false); \
}
BASE_FLOAT_ARITHMETIC_STUBS(Add)
@@ -341,17 +361,17 @@ BASE_FLOAT_ARITHMETIC_STUBS(Mod)
#undef BASE_FLOAT_ARITHMETIC_STUBS
template <typename T>
-typename std::enable_if<std::numeric_limits<T>::is_iec559, T>::type CheckedNeg(
- T value,
- bool*) {
- return static_cast<T>(-value);
+typename std::enable_if<std::numeric_limits<T>::is_iec559, bool>::type
+CheckedNeg(T value, T* result) {
+ *result = static_cast<T>(-value);
+ return true;
}
template <typename T>
-typename std::enable_if<std::numeric_limits<T>::is_iec559, T>::type CheckedAbs(
- T value,
- bool*) {
- return static_cast<T>(std::abs(value));
+typename std::enable_if<std::numeric_limits<T>::is_iec559, bool>::type
+CheckedAbs(T value, T* result) {
+ *result = static_cast<T>(std::abs(value));
+ return true;
}
// Floats carry around their validity state with them, but integers do not. So,
« no previous file with comments | « base/numerics/safe_math.h ('k') | base/numerics/safe_numerics_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698