diff --git a/src/core/include/mp-units/safe_int.h b/src/core/include/mp-units/safe_int.h index 11b215c77..0f9668de7 100644 --- a/src/core/include/mp-units/safe_int.h +++ b/src/core/include/mp-units/safe_int.h @@ -140,7 +140,14 @@ template template [[nodiscard]] constexpr bool mul_overflows(T lhs, T rhs) noexcept { - if constexpr (integer_rep_width_v < max_native_width) { + // Use a wider type for the overflow check whenever one exists. On platforms without native + // `__int128`, `int128_t` is the synthetic `double_width_int` — still a usable + // 128-bit type — so `max_native_width` (which only counts builtin widths) is the wrong + // ceiling; check directly against `integer_rep_width_v` instead. The else branch + // below assumes T is the widest available signed integer (i.e. (u)int128_t) and computes + // `max` by truncating uint128_t — that truncation is what produced the bug on MSVC for + // T == long long. + if constexpr (integer_rep_width_v < integer_rep_width_v) { using wide = double_width_int_for_t; const wide product = static_cast(lhs) * static_cast(rhs); return product > static_cast(std::numeric_limits::max()) || @@ -336,6 +343,65 @@ struct safe_int_binary_ops { if (std::cmp_greater(lhs.value_, rhs.value_)) return std::strong_ordering::greater; return std::strong_ordering::equal; } + + // Heterogeneous arithmetic: safe_int op safe_int, T != U, matching sign. + // Same idiom as the comparison operators above — ADL finds these via the base class, + // the homogeneous (T == U) inline-friend operators inside `safe_int` win for same-T + // calls because non-template beats template in overload resolution, and the + // heterogeneous calls have no competition. Resolves the MSVC C2666 ambiguity on + // `safe_int + safe_int` when `int` and `long` have equal width (Windows). + template + requires(!std::same_as && same_sign_v) + [[nodiscard]] friend constexpr auto operator+(safe_int lhs, safe_int rhs) + -> safe_int, EP> + { + using R = integral_op_result_t; + if (add_overflows(static_cast(lhs.value_), static_cast(rhs.value_))) + EP::on_overflow("safe_int: addition overflow"); + return lhs.value_ + rhs.value_; + } + + template + requires(!std::same_as && same_sign_v) + [[nodiscard]] friend constexpr auto operator-(safe_int lhs, safe_int rhs) + -> safe_int, EP> + { + using R = integral_op_result_t; + if (sub_overflows(static_cast(lhs.value_), static_cast(rhs.value_))) + EP::on_overflow("safe_int: subtraction overflow"); + return lhs.value_ - rhs.value_; + } + + template + requires(!std::same_as && same_sign_v) + [[nodiscard]] friend constexpr auto operator*(safe_int lhs, safe_int rhs) + -> safe_int, EP> + { + using R = integral_op_result_t; + if (mul_overflows(static_cast(lhs.value_), static_cast(rhs.value_))) + EP::on_overflow("safe_int: multiplication overflow"); + return lhs.value_ * rhs.value_; + } + + template + requires(!std::same_as && same_sign_v) + [[nodiscard]] friend constexpr auto operator/(safe_int lhs, safe_int rhs) + -> safe_int, EP> + { + using R = integral_op_result_t; + if (div_overflows(static_cast(lhs.value_), static_cast(rhs.value_))) + EP::on_overflow("safe_int: division overflow"); + return lhs.value_ / rhs.value_; + } + + template + requires(!std::same_as && same_sign_v) + [[nodiscard]] friend constexpr auto operator%(safe_int lhs, safe_int rhs) + -> safe_int, EP> + { + if (rhs.value_ == U{0}) EP::on_overflow("safe_int: modulo by zero"); + return lhs.value_ % rhs.value_; + } }; } // namespace detail diff --git a/test/static/safe_int_test.cpp b/test/static/safe_int_test.cpp index 9c206ae24..e684450d2 100644 --- a/test/static/safe_int_test.cpp +++ b/test/static/safe_int_test.cpp @@ -617,6 +617,41 @@ static_assert(safe_int{6} * 2LL == safe_int{12}); static_assert(2LL * safe_int{6} == safe_int{12}); static_assert(safe_int{12} / 4LL == safe_int{3}); +// ============================================================================ +// Cross-type integral — additional convertibility coverage +// +// Exercises the `safe_int op integral_U` overloads for combinations not covered above: +// - narrower raw RHS (promotes to T's rank, stays safe_int) +// - `long` (whose width differs across platforms — 32-bit on Windows, 64-bit on POSIX), +// verifying that the safe_int op integral overload is chosen over implicit conversion +// of the raw integer to safe_int via the converting ctor. +// ============================================================================ + +// Narrowing raw RHS: result keeps the wider wrapper type. +// short → int by C++ promotion rules, so safe_int + short → safe_int. +static_assert(std::is_same_v{} + short{1}), safe_int>); +static_assert(std::is_same_v{}), safe_int>); +static_assert(safe_int{3} + short{4} == safe_int{7}); +static_assert(short{4} + safe_int{3} == safe_int{7}); + +// safe_int + int → safe_int (raw RHS has higher rank). +static_assert(std::is_same_v{} + 1), safe_int>); +static_assert(std::is_same_v{}), safe_int>); +static_assert(safe_int{3} + 4 == safe_int{7}); + +// safe_int + 1L: result is safe_int. On Windows where `long` and `int` have +// equal width this would otherwise be tempted by the converting ctor (raw long → safe_int), +// which would yield safe_int; the safe_int op integral overload wins by exact match. +static_assert(std::is_same_v{} + 1L), safe_int>); +static_assert(std::is_same_v{}), safe_int>); +static_assert(safe_int{3} + 4L == safe_int{7L}); +static_assert(4L + safe_int{3} == safe_int{7L}); + +// Unsigned widening: safe_int + uint8_t → safe_int. +static_assert(std::is_same_v{} + std::uint8_t{1}), safe_int>); +static_assert(std::is_same_v{}), safe_int>); +static_assert(safe_int{3u} + std::uint8_t{4} == safe_int{7u}); + // int × unsigned scalar arithmetic is intentionally ill-formed — same rationale as // safe_int + safe_int: sign-mismatch conversions produce counterintuitive // results (e.g. safe_int{-1} * 2u → UINT_MAX-1 via reinterpretation). @@ -763,9 +798,12 @@ static_assert(safe_int{3} * safe_int{4L} == safe_int{12L}); static_assert(safe_int{12} / safe_int{4L} == safe_int{3L}); static_assert(safe_int{10} % safe_int{3L} == safe_int{1L}); -// Widening prevents overflow: INT_MAX fits in long, so no overflow here -static_assert(safe_int{std::numeric_limits::max()} + safe_int{1L} == - safe_int{static_cast(std::numeric_limits::max()) + 1L}); +// Widening prevents overflow: INT_MAX + 1 fits in long long on every platform. +// (`long long` rather than `long`: on Windows `long` is 32-bit and equal to `int`, so the +// RHS `static_cast(INT_MAX) + 1L` would itself be 32-bit signed overflow — UB at +// constexpr — and the assertion would not be portable.) +static_assert(safe_int{std::numeric_limits::max()} + safe_int{1LL} == + safe_int{static_cast(std::numeric_limits::max()) + 1LL}); // Heterogeneous comparison (operator== and operator<=>) static_assert(safe_int{3} == safe_int{3L});