Add ClampedNumeric templates

base/numerics/checked_math_impl.h

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_NUMERICS_CHECKED_MATH_IMPL_H_
 #define BASE_NUMERICS_CHECKED_MATH_IMPL_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 164 matching lines @@
         (IntegerBitsPlusSign<__typeof__(x * y)>::value ==
              IntegerBitsPlusSign<intptr_t>::value &&
          std::is_signed<T>::value == std::is_signed<U>::value);
 #else
     static const bool kUseMaxInt = true;
 #endif
     if (kUseMaxInt)
       return !__builtin_mul_overflow(x, y, result);
 #endif
     using Promotion = typename FastIntegerArithmeticPromotion<T, U>::type;
+    // Verify the destination type can hold the result (always true for 0).
+    if (!(IsValueInRangeForNumericType<Promotion>(x) &&
+          IsValueInRangeForNumericType<Promotion>(y)) &&
+        x && y) {
+      return false;

[jschuh, 2017/06/26 20:26:50]

The only functional change here is supporting the case where either operand is
zero, which is hit by some of the ClampedNumeric uses.

+    }
     Promotion presult;
-    // Fail if either operand is out of range for the promoted type.
-    // TODO(jschuh): This could be made to work for a broader range of values.
-    bool is_valid = IsValueInRangeForNumericType<Promotion>(x) &&
-                    IsValueInRangeForNumericType<Promotion>(y);
-
+    bool is_valid = true;
     if (IsIntegerArithmeticSafe<Promotion, T, U>::value) {
       presult = static_cast<Promotion>(x) * static_cast<Promotion>(y);
     } else {
-      is_valid &= CheckedMulImpl(static_cast<Promotion>(x),
-                                 static_cast<Promotion>(y), &presult);
+      is_valid = CheckedMulImpl(static_cast<Promotion>(x),
+                                static_cast<Promotion>(y), &presult);
     }
     *result = static_cast<V>(presult);
     return is_valid && IsValueInRangeForNumericType<V>(presult);
   }
 };
 
 // Avoid poluting the namespace once we're done with the macro.
 #undef USE_OVERFLOW_BUILTINS
 
 // Division just requires a check for a zero denominator or an invalid negation
@@ skipping 29 matching lines @@
     is_valid &= CheckedDivImpl(static_cast<Promotion>(x),
                                static_cast<Promotion>(y), &presult);
     *result = static_cast<V>(presult);
     return is_valid && IsValueInRangeForNumericType<V>(presult);
   }
 };
 
 template <typename T>
 bool CheckedModImpl(T x, T y, T* result) {
   static_assert(std::is_integral<T>::value, "Type must be integral");
-  if (y > 0) {
+  if (y) {
     *result = static_cast<T>(x % y);
     return true;
   }
   return false;
 }
 
 template <typename T, typename U, class Enable = void>
 struct CheckedModOp {};
 
 template <typename T, typename U>
 struct CheckedModOp<T,
                     U,
                     typename std::enable_if<std::is_integral<T>::value &&
                                             std::is_integral<U>::value>::type> {
   using result_type = typename MaxExponentPromotion<T, U>::type;
   template <typename V>
   static bool Do(T x, U y, V* result) {
     using Promotion = typename BigEnoughPromotion<T, U>::type;
     Promotion presult;
     bool is_valid = CheckedModImpl(static_cast<Promotion>(x),
                                    static_cast<Promotion>(y), &presult);
     *result = static_cast<V>(presult);
     return is_valid && IsValueInRangeForNumericType<V>(presult);
   }
 };
 
 template <typename T, typename U, class Enable = void>
 struct CheckedLshOp {};
 
-// Left shift. Shifts less than 0 or greater than or equal to the number
-// of bits in the promoted type are undefined. Shifts of negative values
-// are undefined. Otherwise it is defined when the result fits.
 template <typename T, typename U>
 struct CheckedLshOp<T,
                     U,
                     typename std::enable_if<std::is_integral<T>::value &&
                                             std::is_integral<U>::value>::type> {
   using result_type = T;
   template <typename V>
   static bool Do(T x, U shift, V* result) {
-    using ShiftType = typename std::make_unsigned<T>::type;
-    static const ShiftType kBitWidth = IntegerBitsPlusSign<T>::value;
-    const ShiftType real_shift = static_cast<ShiftType>(shift);
-    // Signed shift is not legal on negative values.
-    if (!IsValueNegative(x) && real_shift < kBitWidth) {
+    if (!IsValueNegative(x) &&
+        as_unsigned(shift) < std::numeric_limits<T>::digits) {
       // Just use a multiplication because it's easy.
       // TODO(jschuh): This could probably be made more efficient.
-      if (!std::is_signed<T>::value || real_shift != kBitWidth - 1)
-        return CheckedMulOp<T, T>::Do(x, static_cast<T>(1) << shift, result);
-      return !x;  // Special case zero for a full width signed shift.
+      return CheckedMulOp<T, T>::Do(x, T(1) << shift, result);
     }
-    return false;
+    // The standard defines only the result of the shift, rather than the
+    // values of source operands. So, shifting zero by any amount is legal.
+    // Otherwise, we overflowed or have an undefined operation.
+    return !x ? (*result = V(0)), true : false;

[jschuh, 2017/06/26 20:26:50]

Stumbled on a spec error in how I was handling zero, so I chose to clean it up
in this CL in order to match the ClampedNumeric behavior.

[dcheng, 2017/06/28 07:25:06]

Acknowledged--though for future CLs, it would be nice to split fixes like this
out into a separate CL (since it makes it easier to review any associated test
changes). Dependent CLs are really easy in Gerrit ;-)

That being said... here's what I found in the C++11 spec in [expr.shift]/1:
The behavior is undefined if the right operand is negative, or greater than or
equal to the length in bits of the promoted left operand.

I don't see any caveat here for 0?

[jschuh, 2017/06/28 12:32:37]

Weird. I'm looking at it now and you're right. Maybe I was looking at a working
draft that was later changed. I'll fix it and the associated tests.

   }
 };
 
 template <typename T, typename U, class Enable = void>
 struct CheckedRshOp {};
 
-// Right shift. Shifts less than 0 or greater than or equal to the number
-// of bits in the promoted type are undefined. Otherwise, it is always defined,
-// but a right shift of a negative value is implementation-dependent.
 template <typename T, typename U>
 struct CheckedRshOp<T,
                     U,
                     typename std::enable_if<std::is_integral<T>::value &&
                                             std::is_integral<U>::value>::type> {
   using result_type = T;
   template <typename V>
   static bool Do(T x, U shift, V* result) {
     // Use the type conversion push negative values out of range.
     using ShiftType = typename std::make_unsigned<T>::type;
-    if (static_cast<ShiftType>(shift) < IntegerBitsPlusSign<T>::value) {
+    if (!x || static_cast<ShiftType>(shift) < IntegerBitsPlusSign<T>::value) {

[jschuh, 2017/06/26 20:26:50]

Stumbled on a spec error in how I was handling zero, so I had to clean it up
because the ClampedNumeric implementation relies on this function.

[dcheng, 2017/06/28 07:25:06]

Ditto.

[jschuh, 2017/06/28 12:32:37]

Yup.

       T tmp = x >> shift;
       *result = static_cast<V>(tmp);
       return IsValueInRangeForNumericType<V>(tmp);
     }
     return false;
   }
 };
 
 template <typename T, typename U, class Enable = void>
 struct CheckedAndOp {};
@@ skipping 236 matching lines @@
     return value_ <= std::numeric_limits<T>::max() &&
            value_ >= std::numeric_limits<T>::lowest();
   }
   constexpr T value() const { return value_; }
 };
 
 }  // namespace internal
 }  // namespace base
 
 #endif  // BASE_NUMERICS_CHECKED_MATH_IMPL_H_