Skip to content

Commit 9a7c6fc

Browse files
committed
Merge branch 'gh47926-safe-arith' into exp-csv-fuzz-safe-arith
2 parents a67cc55 + e5ae2cf commit 9a7c6fc

File tree

2 files changed

+29
-23
lines changed

2 files changed

+29
-23
lines changed

cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,10 @@ class TestUnaryArithmetic : public TestBaseUnaryArithmetic<T, ArithmeticOptions>
241241
protected:
242242
using Base = TestBaseUnaryArithmetic<T, ArithmeticOptions>;
243243
using Base::options_;
244-
void SetOverflowCheck(bool value) { options_.check_overflow = value; }
244+
void SetOverflowCheck(bool value) {
245+
ARROW_SCOPED_TRACE("check_overflow = ", value);
246+
options_.check_overflow = value;
247+
}
245248
};
246249

247250
template <typename T>
@@ -421,7 +424,10 @@ class TestBinaryArithmetic : public TestBaseArithmetic<T> {
421424
AssertArraysApproxEqual(*expected, *actual, /*verbose=*/true, equal_options_);
422425
}
423426

424-
void SetOverflowCheck(bool value = true) { options_.check_overflow = value; }
427+
void SetOverflowCheck(bool value) {
428+
ARROW_SCOPED_TRACE("check_overflow = ", value);
429+
options_.check_overflow = value;
430+
}
425431

426432
void SetNansEqual(bool value = true) {
427433
this->equal_options_ = equal_options_.nans_equal(value);

cpp/src/arrow/util/int_util_overflow.h

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,15 @@ namespace internal {
4040
return !check_##_op_name##_##_type##_##_type(u, v, out); \
4141
}
4242

43-
#define SAFE_INT_OPS_WITH_OVERFLOW(_func_name, _psnip_op) \
44-
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _psnip_op, int32_t, int32) \
45-
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _psnip_op, int64_t, int64) \
46-
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _psnip_op, uint32_t, uint32) \
47-
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _psnip_op, uint64_t, uint64)
43+
#define SAFE_INT_OPS_WITH_OVERFLOW(_func_name, _op_name) \
44+
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, int32_t, int32) \
45+
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, int64_t, int64) \
46+
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, uint32_t, uint32) \
47+
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, uint64_t, uint64)
4848

4949
SAFE_INT_OPS_WITH_OVERFLOW(SafeIntAddWithOverflow, add)
5050
SAFE_INT_OPS_WITH_OVERFLOW(SafeIntSubtractWithOverflow, sub)
5151
SAFE_INT_OPS_WITH_OVERFLOW(SafeIntMultiplyWithOverflow, mul)
52-
SAFE_INT_OPS_WITH_OVERFLOW(SafeIntDivideWithOverflow, div)
5352

5453
#undef SAFE_INT_OP_WITH_OVERFLOW
5554
#undef SAFE_INT_OPS_WITH_OVERFLOW
@@ -77,8 +76,8 @@ template <typename Int>
7776
return __builtin_add_overflow(u, v, out);
7877
#else
7978
if constexpr (sizeof(Int) < 4) {
80-
auto r =
81-
static_cast<upscaled_int32_t<Int> >(u) + static_cast<upscaled_int32_t<Int> >(v);
79+
using UpscaledInt = upscaled_int32_t<Int>;
80+
auto r = static_cast<UpscaledInt>(u) + static_cast<UpscaledInt>(v);
8281
*out = static_cast<Int>(r);
8382
return r != *out;
8483
} else {
@@ -93,8 +92,8 @@ template <typename Int>
9392
return __builtin_sub_overflow(u, v, out);
9493
#else
9594
if constexpr (sizeof(Int) < 4) {
96-
auto r =
97-
static_cast<upscaled_int32_t<Int> >(u) - static_cast<upscaled_int32_t<Int> >(v);
95+
using UpscaledInt = upscaled_int32_t<Int>;
96+
auto r = static_cast<UpscaledInt>(u) - static_cast<UpscaledInt>(v);
9897
*out = static_cast<Int>(r);
9998
return r != *out;
10099
} else {
@@ -109,8 +108,8 @@ template <typename Int>
109108
return __builtin_mul_overflow(u, v, out);
110109
#else
111110
if constexpr (sizeof(Int) < 4) {
112-
auto r =
113-
static_cast<upscaled_int32_t<Int> >(u) * static_cast<upscaled_int32_t<Int> >(v);
111+
using UpscaledInt = upscaled_int32_t<Int>;
112+
auto r = static_cast<UpscaledInt>(u) * static_cast<UpscaledInt>(v);
114113
*out = static_cast<Int>(r);
115114
return r != *out;
116115
} else {
@@ -122,17 +121,18 @@ template <typename Int>
122121
template <typename Int>
123122
[[nodiscard]] bool DivideWithOverflowGeneric(Int u, Int v, Int* out) {
124123
if (v == 0) {
124+
*out = Int{};
125125
return true;
126126
}
127-
if constexpr (sizeof(Int) < 4) {
128-
using UpscaledInt = upscaled_int32_t<Int>;
129-
UpscaledInt r;
130-
bool error = SafeIntDivideWithOverflow(UpscaledInt{u}, UpscaledInt{v}, &r);
131-
*out = static_cast<Int>(r);
132-
return error || r != *out;
133-
} else {
134-
return SafeIntDivideWithOverflow(u, v, out);
127+
if constexpr (std::is_signed_v<Int>) {
128+
constexpr auto kMin = std::numeric_limits<Int>::min();
129+
if (u == kMin && v == -1) {
130+
*out = kMin;
131+
return true;
132+
}
135133
}
134+
*out = u / v;
135+
return false;
136136
}
137137

138138
// Define non-generic versions of the above so as to benefit from automatic
@@ -141,7 +141,7 @@ template <typename Int>
141141

142142
#define NON_GENERIC_OP_WITH_OVERFLOW(_func_name, _c_type) \
143143
[[nodiscard]] inline bool _func_name(_c_type u, _c_type v, _c_type* out) { \
144-
return _func_name##Generic(u, v, out); \
144+
return ARROW_PREDICT_FALSE(_func_name##Generic(u, v, out)); \
145145
}
146146

147147
#define NON_GENERIC_OPS_WITH_OVERFLOW(_func_name) \

0 commit comments

Comments
 (0)