Fix silent truncations when extracting values from CheckedNumeric

base/numerics/safe_conversions.h

Index: base/numerics/safe_conversions.h
diff --git a/base/numerics/safe_conversions.h b/base/numerics/safe_conversions.h
index 4b9129dfffe94329c954bc1842069e7e5bd1e919..71408102eabc4843493cf46e086129c766dcce8e 100644
--- a/base/numerics/safe_conversions.h
+++ b/base/numerics/safe_conversions.h
@@ -9,12 +9,40 @@
 
 #include <cassert>
 #include <limits>
+#include <ostream>
 #include <type_traits>
 
 #include "base/numerics/safe_conversions_impl.h"
 
 namespace base {
 
+// The following are helper constexpr template functions and classes for safely
+// performing a range of conversions, assignments, and tests:
+//
+//  checked_cast<> - Analogous to static_cast<> for numeric types, except
+//      that it CHECKs that the specified numeric conversion will not overflow
+//      or underflow. NaN source will always trigger a CHECK.
+//      The default CHECK triggers a crash, but the handler can be overriden.
+//  saturated_cast<> - Analogous to static_cast<> for numeric types, except
+//      that it returns a saturated result when the specified numeric conversion
+//      would otherwise overflow or underflow. An NaN source returns 0 by
+//      default, but can be overridden to return a different result.
+//  strict_cast<> - Analogous to static_cast<> for numeric types, except that
+//      it will cause a compile failure if the destination type is not large
+//      enough to contain any value in the source type. It performs no runtime
+//      checking and thus introduces no runtime overhead.
+//  IsValueInRangeForNumericType<>() - A convenience function that returns true
+//      if the type supplied to the template parameter can represent the value
+//      passed as an argument to the function.
+//  IsValueNegative<>() - A convenience function that will accept any arithmetic
+//      type as an argument and will return whether the value is less than zero.
+//      Unsigned types always return false.
+//  StrictNumeric<> - A wrapper type that performs assignments and copies via
+//      the strict_cast<> template, and can perform valid arithmetic comparisons
+//      across any range of arithmetic types. StrictNumeric is the return type
+//      for values extracted from a CheckedNumeric class instance. The raw
+//      arithmetic value is extracted via static_cast to the underlying type.
+
 // Convenience function that returns true if the supplied value is in range
 // for the destination type.
 template <typename Dst, typename Src>
@@ -63,8 +91,9 @@ template <typename Dst,
 constexpr Dst checked_cast(Src value) {
   // This throws a compile-time error on evaluating the constexpr if it can be
   // determined at compile-time as failing, otherwise it will CHECK at runtime.
-  return IsValueInRangeForNumericType<Dst>(value)
-             ? static_cast<Dst>(value)
+  using SrcType = typename internal::UnderlyingType<Src>::type;
+  return IsValueInRangeForNumericType<Dst, SrcType>(value)
+             ? static_cast<Dst>(static_cast<SrcType>(value))
              : CheckHandler::template HandleFailure<Dst>();
 }
 
@@ -91,7 +120,6 @@ constexpr Dst saturated_cast_impl(const Src value,
                            ? std::numeric_limits<Dst>::max()
                            : NaNHandler::template HandleFailure<Dst>()));
 }
-}  // namespace internal
 
 // saturated_cast<> is analogous to static_cast<> for numeric types, except
 // that the specified numeric conversion will saturate rather than overflow or
@@ -101,10 +129,13 @@ template <typename Dst,
           class NaNHandler = SaturatedCastNaNBehaviorReturnZero,
           typename Src>
 constexpr Dst saturated_cast(Src value) {
+  using SrcType = typename UnderlyingType<Src>::type;
   return std::numeric_limits<Dst>::is_iec559
-             ? static_cast<Dst>(value)  // Floating point optimization.
+             ? static_cast<Dst>(
+                   static_cast<SrcType>(value))  // Floating point optimization.
              : internal::saturated_cast_impl<Dst, NaNHandler>(
-                   value, internal::DstRangeRelationToSrcRange<Dst>(value));
+                   value,
+                   internal::DstRangeRelationToSrcRange<Dst, SrcType>(value));
 }
 
 // strict_cast<> is analogous to static_cast<> for numeric types, except that
@@ -112,22 +143,41 @@ constexpr Dst saturated_cast(Src value) {
 // to contain any value in the source type. It performs no runtime checking.
 template <typename Dst, typename Src>
 constexpr Dst strict_cast(Src value) {
-  static_assert(std::numeric_limits<Src>::is_specialized,
-                "Argument must be numeric.");
+  using SrcType = typename UnderlyingType<Src>::type;
+  static_assert(UnderlyingType<Src>::is_numeric, "Argument must be numeric.");
   static_assert(std::numeric_limits<Dst>::is_specialized,
                 "Result must be numeric.");
-  static_assert((internal::StaticDstRangeRelationToSrcRange<Dst, Src>::value ==
-                 internal::NUMERIC_RANGE_CONTAINED),
-                "The numeric conversion is out of range for this type. You "
-                "should probably use one of the following conversion "
-                "mechanisms on the value you want to pass:\n"
-                "- base::checked_cast\n"
-                "- base::saturated_cast\n"
-                "- base::CheckedNumeric");
-
-  return static_cast<Dst>(value);
+
+  // If you got here from a compiler error, it's because you tried to assign
+  // from a source type to a destination type that has insufficient range.
+  // The solution may be to change the destination type you're assigning to,
+  // and use one large enough to represent the source.
+  // Alternatively, you may be better served with the checked_cast<> or
+  // saturated_cast<> template functions for your particular use case.
+  static_assert(StaticDstRangeRelationToSrcRange<Dst, SrcType>::value ==
+                    NUMERIC_RANGE_CONTAINED,
+                "The source type is out of range for the destination type. "
+                "Please see strict_cast<> comments for more information.");
+
+  return static_cast<Dst>(static_cast<SrcType>(value));
 }
 
+// Some wrappers to statically check that a type is in range.
+template <typename Dst, typename Src, class Enable = void>
+struct IsNumericRangeContained {
+  static const bool value = false;
+};
+
+template <typename Dst, typename Src>
+struct IsNumericRangeContained<
+    Dst,
+    Src,
+    typename std::enable_if<ArithmeticOrUnderlyingEnum<Dst>::value &&
+                            ArithmeticOrUnderlyingEnum<Src>::value>::type> {
+  static const bool value = StaticDstRangeRelationToSrcRange<Dst, Src>::value ==
+                            NUMERIC_RANGE_CONTAINED;
+};
+
 // StrictNumeric implements compile time range checking between numeric types by
 // wrapping assignment operations in a strict_cast. This class is intended to be
 // used for function arguments and return types, to ensure the destination type
@@ -153,19 +203,62 @@ class StrictNumeric {
   // This is not an explicit constructor because we implicitly upgrade regular
   // numerics to StrictNumerics to make them easier to use.
   template <typename Src>
-  constexpr StrictNumeric(Src value)
+  constexpr StrictNumeric(Src value)  // NOLINT(runtime/explicit)
       : value_(strict_cast<T>(value)) {}
 
-  // The numeric cast operator basically handles all the magic.
-  template <typename Dst>
+  // If you got here from a compiler error, it's because you tried to assign
+  // from a source type to a destination type that has insufficient range.
+  // The solution may be to change the destination type you're assigning to,
+  // and use one large enough to represent the source.
+  // If you're assigning from a CheckedNumeric<> class, you may be able to use
+  // the AssignIfValid() member function, specify a narrower destination type to
+  // the member value functions (e.g. val.template ValueOrDie<Dst>()), use one
+  // of the value helper functions (e.g. ValueOrDieForType<Dst>(val)).
+  // If you've encountered an _ambiguous overload_ you can use a static_cast<>
+  // to explicitly cast the result to the destination type.
+  // If none of that works, you may be better served with the checked_cast<> or
+  // saturated_cast<> template functions for your particular use case.
+  template <typename Dst,
+            typename std::enable_if<
+                IsNumericRangeContained<Dst, T>::value>::type* = nullptr>
   constexpr operator Dst() const {
-    return strict_cast<Dst>(value_);
+    return static_cast<typename ArithmeticOrUnderlyingEnum<Dst>::type>(value_);
   }
 
  private:
   const T value_;
 };
 
+// Overload the ostream output operator to make logging work nicely.
+template <typename T>
+std::ostream& operator<<(std::ostream& os, const StrictNumeric<T>& value) {
+  os << static_cast<T>(value);
+  return os;
+}
+
+#define STRICT_COMPARISON_OP(NAME, OP)                               \
+  template <typename L, typename R,                                  \
+            typename std::enable_if<                                 \
+                internal::IsStrictOp<L, R>::value>::type* = nullptr> \
+  constexpr bool operator OP(const L lhs, const R rhs) {             \
+    return SafeCompare<NAME, typename UnderlyingType<L>::type,       \
+                       typename UnderlyingType<R>::type>(lhs, rhs);  \
+  }
+
+STRICT_COMPARISON_OP(IsLess, <);
+STRICT_COMPARISON_OP(IsLessOrEqual, <=);
+STRICT_COMPARISON_OP(IsGreater, >);
+STRICT_COMPARISON_OP(IsGreaterOrEqual, >=);
+STRICT_COMPARISON_OP(IsEqual, ==);
+STRICT_COMPARISON_OP(IsNotEqual, !=);
+
+#undef STRICT_COMPARISON_OP
+};
+
+using internal::strict_cast;
+using internal::saturated_cast;
+using internal::StrictNumeric;
+
 // Explicitly make a shorter size_t typedef for convenience.
 typedef StrictNumeric<size_t> SizeT;