Add variadic helper functions for CheckedNumeric math operations

base/numerics/safe_math.h

Index: base/numerics/safe_math.h
diff --git a/base/numerics/safe_math.h b/base/numerics/safe_math.h
index 470f4666981cc3788f85e7137807e31b4a1a79ec..dea944c34768f65deaa2bcf0ed6efdb8b0713e55 100644
--- a/base/numerics/safe_math.h
+++ b/base/numerics/safe_math.h
@@ -13,7 +13,6 @@
 #include "base/numerics/safe_math_impl.h"
 
 namespace base {
-
 namespace internal {
 
 // CheckedNumeric implements all the logic and operators for detecting integer
@@ -28,9 +27,21 @@ namespace internal {
 // ValueOrDie() - Returns the underlying value. If the state is not valid this
 //                call will crash on a CHECK.
 // ValueOrDefault() - Returns the current value, or the supplied default if the
-//                    state is not valid.
+//                    state is not valid (will not trigger a CHECK).
 // ValueFloating() - Returns the underlying floating point value (valid only
-//                   only for floating point CheckedNumeric types).
+//                   only for floating point CheckedNumeric types; will not
+//                   trigger a CHECK).
+//
+// You may also use one of the variadic convenience functions, which accept
+// standard arithmetic or CheckedNumeric types, perform arithmetic operations,
+// and return a CheckedNumeric result. The supported functions are:
+//   * CheckedAdd() - addition

[Tom Sepez, 2016/11/23 16:54:58]

Can I explicitly specialize these, for instance if I need an int32_t and I'm
adding a bunch of random crap, eg.

int32_t x = CheckedAdd<int32_t>(....) with int64 types as args?

[jschuh, 2016/11/23 19:18:00]

That's a good idea. I can add it.

[Tom Sepez, 2016/11/23 19:47:08]

I guess I worry about something getting silently promoted to 64-bits, and then
passing that to a smaller function argument.

void frob(int32_t x) {}
int32_t h, w;
...
frob(CheckMul(h, w).ValueOrDefault(0));

Does that give a compilation error? Or is the result type int32_t?  

[Tom Sepez, 2016/11/23 19:47:08]

Yeah, I kinda worry about something getting silently promoted to int64, and then
passing that to an int32 function argument.

[jschuh, 2016/11/23 21:49:22]

Okay, we might have a miscommunication here. I have another CL in progress with
a proxy class to try to catch the scenario you're referring to. However, we've
had that problem the whole time, so this adds no new risk on that front. Also, I
want to honor the natural arithmetic promotions, so I don't want to require an
explicit return type. And it turns out there's no nice way to make it optional
with a function template, so I just added a Cast<T> method to CheckedNumeric,
which serves the same purposes.

+//   * CheckedSub() - subtraction
+//   * CheckedMul() - multiplication
+//   * CheckedDiv() - division
+//   * CheckedMod() - modulous (integer only)
+//   * CheckedLsh() - left integer shift (integer only)
+//   * CheckedRsh() - right integer shift (integer only)
 //
 // Bitwise operations are explicitly not supported, because correct
 // handling of some cases (e.g. sign manipulation) is ambiguous. Comparison
@@ -47,24 +58,6 @@ namespace internal {
 //     Do stuff...
 
 template <typename T>
-class CheckedNumeric;
-
-// Used to treat CheckedNumeric and arithmetic underlying types the same.
-template <typename T>
-struct UnderlyingType {
-  using type = T;
-  static const bool is_numeric = std::is_arithmetic<T>::value;
-  static const bool is_checked = false;
-};
-
-template <typename T>
-struct UnderlyingType<CheckedNumeric<T>> {
-  using type = T;
-  static const bool is_numeric = true;
-  static const bool is_checked = true;
-};
-
-template <typename T>
 class CheckedNumeric {
   static_assert(std::is_arithmetic<T>::value,
                 "CheckedNumeric<T>: T must be a numeric type.");
@@ -197,22 +190,21 @@ class CheckedNumeric {
             typename L,
             typename R>
   static CheckedNumeric MathOp(const L& lhs, const R& rhs) {
-    using Math = M<typename UnderlyingType<L>::type,
-                   typename UnderlyingType<R>::type, void>;
+    using Math = typename MathWrapper<M, L, R>::math;
     T result = 0;
     bool is_valid =
         Wrapper<L>::is_valid(lhs) && Wrapper<R>::is_valid(rhs) &&
-        Math::Op(Wrapper<L>::value(lhs), Wrapper<R>::value(rhs), &result);
+        Math::Do(Wrapper<L>::value(lhs), Wrapper<R>::value(rhs), &result);
     return CheckedNumeric<T>(result, is_valid);
   };
 
   // Assignment arithmetic operations.
   template <template <typename, typename, typename> class M, typename R>
   CheckedNumeric& MathOp(const R& rhs) {
-    using Math = M<T, typename UnderlyingType<R>::type, void>;
+    using Math = typename MathWrapper<M, T, R>::math;
     T result = 0;  // Using T as the destination saves a range check.
     bool is_valid = state_.is_valid() && Wrapper<R>::is_valid(rhs) &&
-                    Math::Op(state_.value(), Wrapper<R>::value(rhs), &result);
+                    Math::Do(state_.value(), Wrapper<R>::value(rhs), &result);
     *this = CheckedNumeric<T>(result, is_valid);
     return *this;
   };
@@ -243,27 +235,76 @@ class CheckedNumeric {
   };
 };
 
+// These variadic templates work out the return types.
+// TODO(jschuh): Rip all this out once we have C++14 non-trailing auto support.
+template <template <typename, typename, typename> class M,
+          typename L,
+          typename R,
+          typename... Args>
+struct ResultType;
+
+template <template <typename, typename, typename> class M,
+          typename L,
+          typename R>
+struct ResultType<M, L, R> {
+  using type = typename MathWrapper<M, L, R>::type;
+};
+
+template <template <typename, typename, typename> class M,
+          typename L,
+          typename R,
+          typename... Args>
+struct ResultType {
+  // The typedef was required here because MSVC fails to compile with "using".

[Tom Sepez, 2016/11/23 16:52:06]

Let's get Bruce to weigh in on this one - at least push the issue upwards.

[jschuh, 2016/11/23 19:18:00]

I've learned that any complex type resolution with using is kinda broken in
MSVS. I've hit it in a few different places this week, and I was planning on
following up with Bruce about it when I got back.

+  typedef
+      typename ResultType<M, typename ResultType<M, L, R>::type, Args...>::type
+          type;
+};
+
+// These implement the variadic wrapper for the math operations.
+template <template <typename, typename, typename> class M,
+          typename L,
+          typename R>
+CheckedNumeric<typename MathWrapper<M, L, R>::type> ChkMathOp(const L lhs,
+                                                              const R rhs) {
+  using Math = typename MathWrapper<M, L, R>::math;
+  return CheckedNumeric<typename Math::result_type>::template MathOp<M>(lhs,
+                                                                        rhs);
+};
+
+template <template <typename, typename, typename> class M,
+          typename L,
+          typename R,
+          typename... Args>
+CheckedNumeric<typename ResultType<M, L, R, Args...>::type>
+ChkMathOp(const L lhs, const R rhs, const Args... args) {
+  auto tmp = ChkMathOp<M>(lhs, rhs);
+  return tmp.IsValid() ? ChkMathOp<M>(tmp, args...)
+                       : decltype(ChkMathOp<M>(tmp, args...))(tmp);
+};
+
 // This is just boilerplate for the standard arithmetic operator overloads.
 // A macro isn't the nicest solution, but it beats rewriting these repeatedly.
-#define BASE_NUMERIC_ARITHMETIC_OPERATORS(NAME, OP, COMPOUND_OP)              \
-  /* Binary arithmetic operator for all CheckedNumeric operations. */         \
-  template <typename L, typename R,                                           \
-            typename std::enable_if<UnderlyingType<L>::is_numeric &&          \
-                                    UnderlyingType<R>::is_numeric &&          \
-                                    (UnderlyingType<L>::is_checked ||         \
-                                     UnderlyingType<R>::is_checked)>::type* = \
-                nullptr>                                                      \
-  CheckedNumeric<                                                             \
-      typename Checked##NAME<typename UnderlyingType<L>::type,                \
-                             typename UnderlyingType<R>::type>::result_type>  \
-  operator OP(const L lhs, const R rhs) {                                     \
-    return decltype(lhs OP rhs)::template MathOp<Checked##NAME>(lhs, rhs);    \
-  }                                                                           \
-  /* Assignment arithmetic operator implementation from CheckedNumeric. */    \
-  template <typename L>                                                       \
-  template <typename R>                                                       \
-  CheckedNumeric<L>& CheckedNumeric<L>::operator COMPOUND_OP(const R rhs) {   \
-    return MathOp<Checked##NAME>(rhs);                                        \
+#define BASE_NUMERIC_ARITHMETIC_OPERATORS(NAME, OP, COMPOUND_OP)               \
+  /* Binary arithmetic operator for all CheckedNumeric operations. */          \
+  template <typename L, typename R,                                            \
+            typename std::enable_if<IsCheckedOp<L, R>::value>::type* =         \
+                nullptr>                                                       \
+  CheckedNumeric<typename MathWrapper<Checked##NAME##Op, L, R>::type>          \
+  operator OP(const L lhs, const R rhs) {                                      \
+    return decltype(lhs OP rhs)::template MathOp<Checked##NAME##Op>(lhs, rhs); \
+  }                                                                            \
+  /* Assignment arithmetic operator implementation from CheckedNumeric. */     \
+  template <typename L>                                                        \
+  template <typename R>                                                        \
+  CheckedNumeric<L>& CheckedNumeric<L>::operator COMPOUND_OP(const R rhs) {    \
+    return MathOp<Checked##NAME##Op>(rhs);                                     \
+  }                                                                            \
+  /* Variadic arithmetic functions that return CheckedNumeric. */              \
+  template <typename L, typename R, typename... Args>                          \
+  CheckedNumeric<typename ResultType<Checked##NAME##Op, L, R, Args...>::type>  \
+      Checked##NAME(const L lhs, const R rhs, const Args... args) {            \
+    return ChkMathOp<Checked##NAME##Op, L, R, Args...>(lhs, rhs, args...);     \
   }
 
 BASE_NUMERIC_ARITHMETIC_OPERATORS(Add, +, +=)
@@ -271,14 +312,21 @@ BASE_NUMERIC_ARITHMETIC_OPERATORS(Sub, -, -=)
 BASE_NUMERIC_ARITHMETIC_OPERATORS(Mul, *, *=)
 BASE_NUMERIC_ARITHMETIC_OPERATORS(Div, /, /=)
 BASE_NUMERIC_ARITHMETIC_OPERATORS(Mod, %, %=)
-BASE_NUMERIC_ARITHMETIC_OPERATORS(LeftShift, <<, <<=)
-BASE_NUMERIC_ARITHMETIC_OPERATORS(RightShift, >>, >>=)
+BASE_NUMERIC_ARITHMETIC_OPERATORS(Lsh, <<, <<=)
+BASE_NUMERIC_ARITHMETIC_OPERATORS(Rsh, >>, >>=)
 
 #undef BASE_NUMERIC_ARITHMETIC_OPERATORS
 
 }  // namespace internal
 
 using internal::CheckedNumeric;
+using internal::CheckedAdd;
+using internal::CheckedSub;
+using internal::CheckedMul;
+using internal::CheckedDiv;
+using internal::CheckedMod;
+using internal::CheckedLsh;
+using internal::CheckedRsh;
 
 }  // namespace base