Cleanup some undefined floating point behavior in base/numerics

base/numerics/safe_math_impl.h

Index: base/numerics/safe_math_impl.h
diff --git a/base/numerics/safe_math_impl.h b/base/numerics/safe_math_impl.h
index 679cfa4e5a0e73c84b541128ed9f9e963b3f2262..b83b1edc4949b826189561ea658342131dcc0de3 100644
--- a/base/numerics/safe_math_impl.h
+++ b/base/numerics/safe_math_impl.h
@@ -635,6 +635,15 @@ class CheckedNumericState<T, NUMERIC_INTEGER> {
   bool is_valid_;
   T value_;
 
+  // Ensures that a type conversion does not trigger undefined behavior.
+  template <typename Src>
+  static constexpr T WellDefinedConversionOrZero(const Src value,
+                                                 const bool is_valid) {
+    using SrcType = typename internal::UnderlyingType<Src>::type;
+    return (std::is_integral<Src>::value || is_valid) ? static_cast<T>(value)
+                                                      : static_cast<T>(0);
+  }
+
  public:
   template <typename Src, NumericRepresentation type>
   friend class CheckedNumericState;
@@ -644,7 +653,7 @@ class CheckedNumericState<T, NUMERIC_INTEGER> {
   template <typename Src>
   constexpr CheckedNumericState(Src value, bool is_valid)
       : is_valid_(is_valid && IsValueInRangeForNumericType<T>(value)),
-        value_(is_valid_ ? static_cast<T>(value) : 0) {
+        value_(WellDefinedConversionOrZero(value, is_valid_)) {
     static_assert(std::is_arithmetic<Src>::value, "Argument must be numeric.");
   }
 
@@ -652,12 +661,12 @@ class CheckedNumericState<T, NUMERIC_INTEGER> {
   template <typename Src>
   constexpr CheckedNumericState(const CheckedNumericState<Src>& rhs)
       : is_valid_(rhs.IsValid()),
-        value_(is_valid_ ? static_cast<T>(rhs.value()) : 0) {}
+        value_(WellDefinedConversionOrZero(rhs.value(), is_valid_)) {}
 
   template <typename Src>
   constexpr explicit CheckedNumericState(Src value)
       : is_valid_(IsValueInRangeForNumericType<T>(value)),
-        value_(is_valid_ ? static_cast<T>(value) : 0) {}
+        value_(WellDefinedConversionOrZero(value, is_valid_)) {}
 
   constexpr bool is_valid() const { return is_valid_; }
   constexpr T value() const { return value_; }
@@ -669,6 +678,18 @@ class CheckedNumericState<T, NUMERIC_FLOATING> {
  private:
   T value_;
 
+  // Ensures that a type conversion does not trigger undefined behavior.
+  template <typename Src>
+  static constexpr T WellDefinedConversionOrNaN(const Src value,
+                                                const bool is_valid) {
+    using SrcType = typename internal::UnderlyingType<Src>::type;
+    return (StaticDstRangeRelationToSrcRange<T, SrcType>::value ==
+                NUMERIC_RANGE_CONTAINED ||
+            is_valid)
+               ? static_cast<T>(value)
+               : static_cast<T>(std::numeric_limits<T>::quiet_NaN());

[Nico, 2016/12/06 17:15:50]

Do you need the cast here? numeric_limits<T>::quiet_NaN() already has type T,
no?

[jschuh, 2016/12/06 17:33:59]

I do not, and that's what I get for hurriedly rewriting from memory.

+  }
+
  public:
   template <typename Src, NumericRepresentation type>
   friend class CheckedNumericState;
@@ -677,18 +698,20 @@ class CheckedNumericState<T, NUMERIC_FLOATING> {
 
   template <typename Src>
   constexpr CheckedNumericState(Src value, bool is_valid)
-      : value_((is_valid && IsValueInRangeForNumericType<T>(value))
-                   ? static_cast<T>(value)
-                   : std::numeric_limits<T>::quiet_NaN()) {}
+      : value_(WellDefinedConversionOrNaN(value, is_valid)) {}
 
   template <typename Src>
   constexpr explicit CheckedNumericState(Src value)
-      : value_(static_cast<T>(value)) {}
+      : value_(WellDefinedConversionOrNaN(
+            value,
+            IsValueInRangeForNumericType<T>(value))) {}
 
   // Copy constructor.
   template <typename Src>
   constexpr CheckedNumericState(const CheckedNumericState<Src>& rhs)
-      : value_(static_cast<T>(rhs.value())) {}
+      : value_(WellDefinedConversionOrNaN(
+            rhs.value(),
+            rhs.is_valid() && IsValueInRangeForNumericType<T>(rhs.value()))) {}
 
   constexpr bool is_valid() const {
     // Written this way because std::isfinite is not reliably constexpr.