Make Checked* functions better align with GCC/Clang intrinsics

base/numerics/safe_math.h

Index: base/numerics/safe_math.h
diff --git a/base/numerics/safe_math.h b/base/numerics/safe_math.h
index 0c88a7c2a30d6efb4597007be46056eb1d2fc999..e2e6c6e5b62c8aa87bd816b7ecf0025ee1843e81 100644
--- a/base/numerics/safe_math.h
+++ b/base/numerics/safe_math.h
@@ -119,24 +119,18 @@ class CheckedNumeric {
   template <typename Src> CheckedNumeric& operator%=(Src rhs);
 
   CheckedNumeric operator-() const {
-    bool is_valid;
-    T value = CheckedNeg(state_.value(), &is_valid);
     // Negation is always valid for floating point.
-    if (std::numeric_limits<T>::is_iec559)
-      return CheckedNumeric<T>(value);
-
-    is_valid &= state_.is_valid();
+    bool is_valid = std::numeric_limits<T>::is_iec559 || IsValid();

[Tom Sepez, 2016/11/07 17:42:57]

nit: maybe flip around these two lines and combine with && expression below.

[jschuh, 2016/11/07 18:16:17]

Done.

+    T value = 0;
+    is_valid = is_valid && CheckedNeg(state_.value(), &value);
     return CheckedNumeric<T>(value, is_valid);
   }
 
   CheckedNumeric Abs() const {
-    bool is_valid;
-    T value = CheckedAbs(state_.value(), &is_valid);
     // Absolute value is always valid for floating point.
-    if (std::numeric_limits<T>::is_iec559)
-      return CheckedNumeric<T>(value);
-
-    is_valid &= state_.is_valid();
+    bool is_valid = std::numeric_limits<T>::is_iec559 || IsValid();
+    T value = 0;
+    is_valid = is_valid && CheckedAbs(state_.value(), &value);
     return CheckedNumeric<T>(value, is_valid);
   }
 
@@ -145,7 +139,7 @@ class CheckedNumeric {
   // of the source, and properly handling signed min.
   CheckedNumeric<typename UnsignedOrFloatForSize<T>::type> UnsignedAbs() const {
     return CheckedNumeric<typename UnsignedOrFloatForSize<T>::type>(
-        CheckedUnsignedAbs(state_.value()), state_.is_valid());
+        SafeUnsignedAbs(state_.value()), state_.is_valid());
   }
 
   CheckedNumeric& operator++() {
@@ -220,15 +214,16 @@ class CheckedNumeric {
     /* Floating point always takes the fast path */                            \
     if (std::numeric_limits<T>::is_iec559)                                     \
       return CheckedNumeric<T>(lhs.ValueUnsafe() OP rhs.ValueUnsafe());        \
+    if (!rhs.IsValid() || !lhs.IsValid())                                      \
+      return CheckedNumeric<Promotion>(0, false);                              \
     if (IsIntegerArithmeticSafe<Promotion, T, T>::value)                       \
-      return CheckedNumeric<Promotion>(lhs.ValueUnsafe() OP rhs.ValueUnsafe(), \
-                                       rhs.IsValid() && lhs.IsValid());        \
-    bool is_valid = true;                                                      \
-    T result = static_cast<T>(                                                 \
+      return CheckedNumeric<Promotion>(lhs.ValueUnsafe()                       \
+                                       OP rhs.ValueUnsafe());                  \
+    Promotion result;                                                          \

[Tom Sepez, 2016/11/07 17:42:57]

nit: want to assign 0 to it?  0 is a valid value of 0 for all uses of 0.

[jschuh, 2016/11/07 18:16:17]

Done.

+    bool is_valid =                                                            \
         Checked##NAME(static_cast<Promotion>(lhs.ValueUnsafe()),               \
-                      static_cast<Promotion>(rhs.ValueUnsafe()), &is_valid));  \
-    return CheckedNumeric<Promotion>(                                          \
-        result, is_valid && lhs.IsValid() && rhs.IsValid());                   \
+                      static_cast<Promotion>(rhs.ValueUnsafe()), &result);     \
+    return CheckedNumeric<Promotion>(result, is_valid);                        \
   }                                                                            \
   /* Assignment arithmetic operator implementation from CheckedNumeric. */     \
   template <typename T>                                                        \
@@ -243,9 +238,10 @@ class CheckedNumeric {
   CheckedNumeric<typename ArithmeticPromotion<T, Src>::type> operator OP(      \
       const CheckedNumeric<Src>& lhs, const CheckedNumeric<T>& rhs) {          \
     typedef typename ArithmeticPromotion<T, Src>::type Promotion;              \
+    if (!rhs.IsValid() || !lhs.IsValid())                                      \
+      return CheckedNumeric<Promotion>(0, false);                              \
     if (IsIntegerArithmeticSafe<Promotion, T, Src>::value)                     \
-      return CheckedNumeric<Promotion>(lhs.ValueUnsafe() OP rhs.ValueUnsafe(), \
-                                       rhs.IsValid() && lhs.IsValid());        \
+      return CheckedNumeric<Promotion>(lhs.ValueUnsafe() OP rhs.ValueUnsafe());\
     return CheckedNumeric<Promotion>::cast(lhs)                                \
         OP CheckedNumeric<Promotion>::cast(rhs);                               \
   }                                                                            \