Cleanup some undefined floating point behavior in base/numerics

base/numerics/safe_math_impl.h

 // Copyright 2014 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_SAFE_MATH_IMPL_H_
 #define BASE_NUMERICS_SAFE_MATH_IMPL_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 617 matching lines @@
 // Integrals require quite a bit of additional housekeeping to manage state.
 template <typename T>
 class CheckedNumericState<T, NUMERIC_INTEGER> {
  private:
   // is_valid_ precedes value_ because member intializers in the constructors
   // are evaluated in field order, and is_valid_ must be read when initializing
   // value_.
   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;
 
   constexpr CheckedNumericState() : is_valid_(true), value_(0) {}
 
   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.");
   }
 
   // Copy constructor.
   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_; }
 };
 
 // Floating points maintain their own validity, but need translation wrappers.
 template <typename T>
 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;
 
   constexpr CheckedNumericState() : value_(0.0) {}
 
   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.
     // TODO(jschuh): Fix this if the libraries ever get fixed.
     return value_ <= std::numeric_limits<T>::max() &&
            value_ >= std::numeric_limits<T>::lowest();
   }
   constexpr T value() const { return value_; }
 };
 
 template <template <typename, typename, typename> class M,
           typename L,
           typename R>
 struct MathWrapper {
   using math = M<typename UnderlyingType<L>::type,
                  typename UnderlyingType<R>::type,
                  void>;
   using type = typename math::result_type;
 };
 
 }  // namespace internal
 }  // namespace base
 
 #endif  // BASE_NUMERICS_SAFE_MATH_IMPL_H_