Cleanup some undefined floating point behavior in base/numerics

base/numerics/safe_conversions.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_CONVERSIONS_H_
 #define BASE_NUMERICS_SAFE_CONVERSIONS_H_
 
 #include <stddef.h>
 
 #include <cassert>
@@ skipping 88 matching lines @@
 
 // HandleNaN will return 0 in this case.
 struct SaturatedCastNaNBehaviorReturnZero {
   template <typename T>
   static constexpr T HandleFailure() {
     return T();
   }
 };
 
 namespace internal {
-// This wrapper is used for C++11 constexpr support by avoiding the declaration
-// of local variables in the saturated_cast template function.
-// TODO(jschuh): convert this back to a switch once we support C++14.
-template <typename Dst, class NaNHandler, typename Src>
+// These wrappers are used for C++11 constexpr support by avoiding both the
+// declaration of local variables and invalid evaluation resulting from the
+// lack of "constexpr if" support in the saturated_cast template function.
+// TODO(jschuh): Convert to single function with a switch once we support C++14.
+template <
+    typename Dst,
+    class NaNHandler,
+    typename Src,
+    typename std::enable_if<std::is_integral<Dst>::value>::type* = nullptr>
 constexpr Dst saturated_cast_impl(const Src value,
                                   const RangeConstraint constraint) {
   return constraint == RANGE_VALID
              ? static_cast<Dst>(value)
              : (constraint == RANGE_UNDERFLOW
                     ? std::numeric_limits<Dst>::lowest()
                     : (constraint == RANGE_OVERFLOW
                            ? std::numeric_limits<Dst>::max()
                            : NaNHandler::template HandleFailure<Dst>()));
 }
 
+template <typename Dst,
+          class NaNHandler,
+          typename Src,
+          typename std::enable_if<std::is_floating_point<Dst>::value>::type* =
+              nullptr>
+constexpr Dst saturated_cast_impl(const Src value,
+                                  const RangeConstraint constraint) {
+  return constraint == RANGE_VALID
+             ? static_cast<Dst>(value)
+             : (constraint == RANGE_UNDERFLOW
+                    ? -std::numeric_limits<Dst>::infinity()

[Nico, 2016/12/06 14:03:37]

I'm assuming you're static_assert()ing somewhere that our floating types satisfy
std::numeric_limits::is_iec559. (in practice, they do, everywhere)

[jschuh, 2016/12/06 15:07:55]

We have a ton of tests that would fail if they didn't.

+                    : (constraint == RANGE_OVERFLOW
+                           ? std::numeric_limits<Dst>::infinity()
+                           : std::numeric_limits<Dst>::quiet_NaN()));
+}
+
 // saturated_cast<> is analogous to static_cast<> for numeric types, except
 // that the specified numeric conversion will saturate rather than overflow or
 // underflow. NaN assignment to an integral will defer the behavior to a
 // specified class. By default, it will return 0.
 template <typename Dst,
           class NaNHandler = SaturatedCastNaNBehaviorReturnZero,
           typename Src>
 constexpr Dst saturated_cast(Src value) {
   using SrcType = typename UnderlyingType<Src>::type;
-  return std::is_floating_point<Dst>::value
-             ? static_cast<Dst>(
-                   static_cast<SrcType>(value))  // Floating point optimization.
-             : internal::saturated_cast_impl<Dst, NaNHandler>(
-                   value,
-                   internal::DstRangeRelationToSrcRange<Dst, SrcType>(value));
+  return internal::saturated_cast_impl<Dst, NaNHandler>(
+      value, internal::DstRangeRelationToSrcRange<Dst, SrcType>(value));
 }
 
 // strict_cast<> is 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.
 template <typename Dst, typename Src>
 constexpr Dst strict_cast(Src value) {
   using SrcType = typename UnderlyingType<Src>::type;
   static_assert(UnderlyingType<Src>::is_numeric, "Argument must be numeric.");
   static_assert(std::is_arithmetic<Dst>::value, "Result must be numeric.");
@@ skipping 116 matching lines @@
 using internal::saturated_cast;
 using internal::StrictNumeric;
 using internal::MakeStrictNum;
 
 // Explicitly make a shorter size_t alias for convenience.
 using SizeT = StrictNumeric<size_t>;
 
 }  // namespace base
 
 #endif  // BASE_NUMERICS_SAFE_CONVERSIONS_H_