Fix perf and constexpr correctness in safe negation

Fixes the following minor issues:
* MSVC silently accepted runtime version of clamped negate in constexpr
  (a correctness issue that also generated marginally slower code).
* Checked negation did not use optimized builtins when available.

Change-Id: Ifd9a756d6620917a915ebdd583909824f2f86410
Reviewed-on: https://chromium-review.googlesource.com/573432
Commit-Queue: Justin Schuh <jschuh@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487267}

base/numerics/checked_math.h

@@ -129,12 +129,18 @@ class CheckedNumeric {
   CheckedNumeric& operator^=(const Src rhs);
 
   constexpr CheckedNumeric operator-() const {
-    return CheckedNumeric<T>(
-        NegateWrapper(state_.value()),
-        IsValid() &&
-            (!std::is_signed<T>::value || std::is_floating_point<T>::value ||
-             NegateWrapper(state_.value()) !=
-                 std::numeric_limits<T>::lowest()));
+    // The negation of two's complement int min is int min, so we simply
+    // check for that in the constexpr case.
+    // We use an optimized code path for a known run-time variable.
+    return MustTreatAsConstexpr(state_.value()) || !std::is_signed<T>::value ||
+                   std::is_floating_point<T>::value
+               ? CheckedNumeric<T>(
+                     NegateWrapper(state_.value()),
+                     IsValid() && (!std::is_signed<T>::value ||
+                                   std::is_floating_point<T>::value ||
+                                   NegateWrapper(state_.value()) !=
+                                       std::numeric_limits<T>::lowest()))
+               : FastRuntimeNegate();
   }
 
   constexpr CheckedNumeric operator~() const {
@@ -239,6 +245,12 @@ class CheckedNumeric {
  private:
   CheckedNumericState<T> state_;
 
+  CheckedNumeric FastRuntimeNegate() const {
+    T result;
+    bool success = CheckedSubOp<T, T>::Do(T(0), state_.value(), &result);
+    return CheckedNumeric<T>(result, IsValid() && success);
+  }
+
   template <typename Src>
   constexpr CheckedNumeric(Src value, bool is_valid)
       : state_(value, is_valid) {}

base/numerics/clamped_math.h

@@ -79,11 +79,11 @@ class ClampedNumeric {
 
   constexpr ClampedNumeric operator-() const {
     return ClampedNumeric<T>(
-        // The negation of two's complement int min is int min, so that's the
-        // only overflow case we have to check for. And in the case of a
-        // run-time variable value_, we can use an optimized code path.
+        // The negation of two's complement int min is int min, so we can
+        // check and just add one to prevent overflow in the constexpr case.
+        // We use an optimized code path for a known run-time variable.
         std::is_signed<T>::value
-            ? (IsCompileTimeConstant(value_)
+            ? (MustTreatAsConstexpr(value_)
                    ? ((std::is_floating_point<T>::value ||
                        NegateWrapper(value_) !=
                            std::numeric_limits<T>::lowest())

base/numerics/safe_conversions_impl.h

@@ -76,15 +76,29 @@ constexpr typename std::make_unsigned<T>::type SafeUnsignedAbs(T value) {
                                 : static_cast<UnsignedT>(value);
 }
 
-// This provides a small optimization that generates more compact code when one
-// of the components in an operation is a compile-time constant.
+// This allows us to switch paths on known compile-time constants.
+#if defined(__clang__) || defined(__GNUC__)
+constexpr bool CanDetectCompileTimeConstant() {
+  return true;
+}
 template <typename T>
 constexpr bool IsCompileTimeConstant(const T v) {
-#if defined(__clang__) || defined(__GNUC__)
   return __builtin_constant_p(v);
+}
 #else
+constexpr bool CanDetectCompileTimeConstant() {
+  return false;
+}
+template <typename T>
+constexpr bool IsCompileTimeConstant(const T v) {
   return false;
+}
 #endif
+template <typename T>
+constexpr bool MustTreatAsConstexpr(const T v) {
+  // Either we can't detect a compile-time constant, and must always use the
+  // constexpr path, or we know we have a compile-time constant.
+  return !CanDetectCompileTimeConstant() || IsCompileTimeConstant(v);
 }
 
 // Forces a crash, like a CHECK(false). Used for numeric boundary errors.