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..20731517e45994b52c46aba24ba5d1183a41c870 100644
--- a/base/numerics/safe_conversions.h
+++ b/base/numerics/safe_conversions.h
@@ -9,12 +9,41 @@
 
 #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.
+//      The default CHECK triggers a crash, but the handler can be overriden.

[Tom Sepez, 2016/11/30 23:58:27]

Nit: can saturated_cast cause a CHECK?  

[jschuh, 2016/12/01 00:58:55]

Nope. Good catch.

+//  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 in the

[Tom Sepez, 2016/11/30 23:58:27]

nit: s/represent in/represent/

[jschuh, 2016/12/01 00:58:55]

Done.

+//      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>
@@ -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
@@ -116,14 +144,21 @@ constexpr Dst strict_cast(Src value) {
                 "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");
+
+  // If you got here from a compiler error, it's because you tried to assign
+  // from a source type a destination type that has insufficient range.

[Tom Sepez, 2016/11/30 23:58:27]

nit: to a destination type.

[jschuh, 2016/12/01 00:58:55]

Done.

+  // The solution may simply be to change the destination type you're assigning

[Tom Sepez, 2016/11/30 23:58:27]

nit: s/simply//

[jschuh, 2016/12/01 00:58:55]

Done.

+  // to, and use one large enough to represent the source.
+  // If you're assigning from a CheckedNumeric<> class, you can use the
+  // AssignIfValid() member function, specify a narrower destination type to the
+  // member value functions (e.g. val.template ValueOrDie<Dst>()), or use one of
+  // the value helper functions (e.g. ValueOrDieForType<Dst>(val)).
+  // Alternatively, you may be better served with the checked_cast<> or
+  // saturated_cast<> template functions for your particular use case.
+  static_assert(internal::StaticDstRangeRelationToSrcRange<Dst, Src>::value ==
+                    internal::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>(value);
 }
@@ -157,15 +192,47 @@ class StrictNumeric {
       : value_(strict_cast<T>(value)) {}
 
   // The numeric cast operator basically handles all the magic.
-  template <typename Dst>
+  template <typename Dst,
+            typename std::enable_if<
+                ArithmeticOrUnderlyingEnum<Dst>::value>::type* = nullptr>
   constexpr operator Dst() const {
-    return strict_cast<Dst>(value_);
+    return strict_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;