Change checked arithmetic functions API to return Optional

Returning optional is much safer.
The previous API had potential to cause use of undefined variables, if
the value passed by pointer was accidentally read afterwards.

Differential Revision: https://reviews.llvm.org/D48137

llvm-svn: 334634

2 files changed, 50 insertions, 67 deletions

diff --git a/llvm/include/llvm/Support/CheckedArithmetic.h b/llvm/include/llvm/Support/CheckedArithmetic.h
index 68bce06..3240487 100644
--- a/llvm/include/llvm/Support/CheckedArithmetic.h
+++ b/llvm/include/llvm/Support/CheckedArithmetic.h
@@ -16,66 +16,61 @@
 #define LLVM_SUPPORT_CHECKEDARITHMETIC_H
 
 #include "llvm/ADT/APInt.h"
+#include "llvm/ADT/Optional.h"
 
 #include <type_traits>
 
 namespace {
 
 /// Utility function to apply a given method of \c APInt \p F to \p LHS and
-/// \p RHS, and write the output into \p Res.
-/// \return Whether the operation overflows.
+/// \p RHS.
+/// \return Empty optional if the operation overflows, or result otherwise.
 template <typename T, typename F>
 typename std::enable_if<std::is_integral<T>::value && sizeof(T) * 8 <= 64,
-                        bool>::type
-checkedOp(T LHS, T RHS, F Op, T *Res = nullptr, bool Signed = true) {
+                        llvm::Optional<T>>::type
+checkedOp(T LHS, T RHS, F Op, bool Signed = true) {
   llvm::APInt ALHS(/*BitSize=*/sizeof(T) * 8, LHS, Signed);
   llvm::APInt ARHS(/*BitSize=*/sizeof(T) * 8, RHS, Signed);
   bool Overflow;
   llvm::APInt Out = (ALHS.*Op)(ARHS, Overflow);
-  if (Res)
-    *Res = Signed ? Out.getSExtValue() : Out.getZExtValue();
-  return Overflow;
+  if (Overflow)
+    return llvm::None;
+  return Signed ? Out.getSExtValue() : Out.getZExtValue();
 }
 }
 
 namespace llvm {
 
-/// Add two signed integers \p LHS and \p RHS, write into \p Res if non-null.
-/// Does not guarantee saturating arithmetic.
-/// \return Whether the result overflows.
+/// Add two signed integers \p LHS and \p RHS, return wrapped result if
+/// available.
 template <typename T>
-typename std::enable_if<std::is_signed<T>::value, bool>::type
-checkedAdd(T LHS, T RHS, T *Res = nullptr) {
-  return checkedOp(LHS, RHS, &llvm::APInt::sadd_ov, Res);
+typename std::enable_if<std::is_signed<T>::value, llvm::Optional<T>>::type
+checkedAdd(T LHS, T RHS) {
+  return checkedOp(LHS, RHS, &llvm::APInt::sadd_ov);
 }
 
-/// Multiply two signed integers \p LHS and \p RHS, write into \p Res if
-/// non-null.
-/// Does not guarantee saturating arithmetic.
-/// \return Whether the result overflows.
+/// Multiply two signed integers \p LHS and \p RHS, return wrapped result
+/// if available.
 template <typename T>
-typename std::enable_if<std::is_signed<T>::value, bool>::type
-checkedMul(T LHS, T RHS, T *Res = nullptr) {
-  return checkedOp(LHS, RHS, &llvm::APInt::smul_ov, Res);
+typename std::enable_if<std::is_signed<T>::value, llvm::Optional<T>>::type
+checkedMul(T LHS, T RHS) {
+  return checkedOp(LHS, RHS, &llvm::APInt::smul_ov);
 }
 
-/// Add two unsigned integers \p LHS and \p RHS, write into \p Res if non-null.
-/// Does not guarantee saturating arithmetic.
-/// \return Whether the result overflows.
+/// Add two unsigned integers \p LHS and \p RHS, return wrapped result
+/// if available.
 template <typename T>
-typename std::enable_if<std::is_unsigned<T>::value, bool>::type
-checkedAddUnsigned(T LHS, T RHS, T *Res = nullptr) {
-  return checkedOp(LHS, RHS, &llvm::APInt::uadd_ov, Res, /*Signed=*/false);
+typename std::enable_if<std::is_unsigned<T>::value, llvm::Optional<T>>::type
+checkedAddUnsigned(T LHS, T RHS) {
+  return checkedOp(LHS, RHS, &llvm::APInt::uadd_ov, /*Signed=*/false);
 }
 
-/// Multiply two unsigned integers \p LHS and \p RHS, write into \p Res if
-/// non-null.
-/// Does not guarantee saturating arithmetic.
-/// \return Whether the result overflows.
+/// Multiply two unsigned integers \p LHS and \p RHS, return wrapped result
+/// if available.
 template <typename T>
-typename std::enable_if<std::is_unsigned<T>::value, bool>::type
-checkedMulUnsigned(T LHS, T RHS, T *Res = nullptr) {
-  return checkedOp(LHS, RHS, &llvm::APInt::umul_ov, Res, /*Signed=*/false);
+typename std::enable_if<std::is_unsigned<T>::value, llvm::Optional<T>>::type
+checkedMulUnsigned(T LHS, T RHS) {
+  return checkedOp(LHS, RHS, &llvm::APInt::umul_ov, /*Signed=*/false);
 }
 
 } // End llvm namespace
diff --git a/llvm/unittests/Support/CheckedArithmeticTest.cpp b/llvm/unittests/Support/CheckedArithmeticTest.cpp
index 7fc6349..f797a9e 100644
--- a/llvm/unittests/Support/CheckedArithmeticTest.cpp
+++ b/llvm/unittests/Support/CheckedArithmeticTest.cpp
@@ -6,65 +6,53 @@ using namespace llvm;
 namespace {
 
 TEST(CheckedArithmetic, CheckedAdd) {
-  int64_t Out;
   const int64_t Max = std::numeric_limits<int64_t>::max();
   const int64_t Min = std::numeric_limits<int64_t>::min();
-  EXPECT_EQ(checkedAdd<int64_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedAdd<int64_t>(Min, -1, &Out), true);
-  EXPECT_EQ(checkedAdd<int64_t>(Max, 1, &Out), true);
-  EXPECT_EQ(checkedAdd<int64_t>(10, 1, &Out), false);
-  EXPECT_EQ(Out, 11);
+  EXPECT_EQ(checkedAdd<int64_t>(Max, Max), None);
+  EXPECT_EQ(checkedAdd<int64_t>(Min, -1), None);
+  EXPECT_EQ(checkedAdd<int64_t>(Max, 1), None);
+  EXPECT_EQ(checkedAdd<int64_t>(10, 1), Optional<int64_t>(11));
 }
 
 TEST(CheckedArithmetic, CheckedAddSmall) {
-  int16_t Out;
   const int16_t Max = std::numeric_limits<int16_t>::max();
   const int16_t Min = std::numeric_limits<int16_t>::min();
-  EXPECT_EQ(checkedAdd<int16_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedAdd<int16_t>(Min, -1, &Out), true);
-  EXPECT_EQ(checkedAdd<int16_t>(Max, 1, &Out), true);
-  EXPECT_EQ(checkedAdd<int16_t>(10, 1, &Out), false);
-  EXPECT_EQ(Out, 11);
+  EXPECT_EQ(checkedAdd<int16_t>(Max, Max), None);
+  EXPECT_EQ(checkedAdd<int16_t>(Min, -1), None);
+  EXPECT_EQ(checkedAdd<int16_t>(Max, 1), None);
+  EXPECT_EQ(checkedAdd<int16_t>(10, 1), Optional<int64_t>(11));
 }
 
 TEST(CheckedArithmetic, CheckedMul) {
-  int64_t Out;
   const int64_t Max = std::numeric_limits<int64_t>::max();
   const int64_t Min = std::numeric_limits<int64_t>::min();
-  EXPECT_EQ(checkedMul<int64_t>(Max, 2, &Out), true);
-  EXPECT_EQ(checkedMul<int64_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedMul<int64_t>(Min, 2, &Out), true);
-  EXPECT_EQ(checkedMul<int64_t>(10, 2, &Out), false);
-  EXPECT_EQ(Out, 20);
+  EXPECT_EQ(checkedMul<int64_t>(Max, 2), None);
+  EXPECT_EQ(checkedMul<int64_t>(Max, Max), None);
+  EXPECT_EQ(checkedMul<int64_t>(Min, 2), None);
+  EXPECT_EQ(checkedMul<int64_t>(10, 2), Optional<int64_t>(20));
 }
 
 TEST(CheckedArithmetic, CheckedMulSmall) {
-  int16_t Out;
   const int16_t Max = std::numeric_limits<int16_t>::max();
   const int16_t Min = std::numeric_limits<int16_t>::min();
-  EXPECT_EQ(checkedMul<int16_t>(Max, 2, &Out), true);
-  EXPECT_EQ(checkedMul<int16_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedMul<int16_t>(Min, 2, &Out), true);
-  EXPECT_EQ(checkedMul<int16_t>(10, 2, &Out), false);
-  EXPECT_EQ(Out, 20);
+  EXPECT_EQ(checkedMul<int16_t>(Max, 2), None);
+  EXPECT_EQ(checkedMul<int16_t>(Max, Max), None);
+  EXPECT_EQ(checkedMul<int16_t>(Min, 2), None);
+  EXPECT_EQ(checkedMul<int16_t>(10, 2), Optional<int16_t>(20));
 }
 
 TEST(CheckedArithmetic, CheckedAddUnsigned) {
-  uint64_t Out;
   const uint64_t Max = std::numeric_limits<uint64_t>::max();
-  EXPECT_EQ(checkedAddUnsigned<uint64_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedAddUnsigned<uint64_t>(Max, 1, &Out), true);
-  EXPECT_EQ(checkedAddUnsigned<uint64_t>(10, 1, &Out), false);
-  EXPECT_EQ(Out, uint64_t(11));
+  EXPECT_EQ(checkedAddUnsigned<uint64_t>(Max, Max), None);
+  EXPECT_EQ(checkedAddUnsigned<uint64_t>(Max, 1), None);
+  EXPECT_EQ(checkedAddUnsigned<uint64_t>(10, 1), Optional<uint64_t>(11));
 }
 
 TEST(CheckedArithmetic, CheckedMulUnsigned) {
-  uint64_t Out;
   const uint64_t Max = std::numeric_limits<uint64_t>::max();
-  EXPECT_EQ(checkedMulUnsigned<uint64_t>(Max, 2, &Out), true);
-  EXPECT_EQ(checkedMulUnsigned<uint64_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedMulUnsigned<uint64_t>(10, 2, &Out), false);
-  EXPECT_EQ(Out, uint64_t(20));
+  EXPECT_EQ(checkedMulUnsigned<uint64_t>(Max, 2), llvm::None);
+  EXPECT_EQ(checkedMulUnsigned<uint64_t>(Max, Max), llvm::None);
+  EXPECT_EQ(checkedMulUnsigned<uint64_t>(10, 2), Optional<uint64_t>(20));
 }