Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<numeric>: check for gcd / lcm overflows #4776

Merged
merged 14 commits into from
Jul 11, 2024
Merged

<numeric>: check for gcd / lcm overflows #4776

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3cee769
`<numeric>`: check for gcd / lcm overflows
AlexGuteniev Jul 1, 2024
7d4bc4d
formal
AlexGuteniev Jul 1, 2024
d9c0c8d
`_Nt`
AlexGuteniev Jul 1, 2024
5633506
check against common type
AlexGuteniev Jul 1, 2024
8c95ecd
actually this was a bogus test
AlexGuteniev Jul 1, 2024
260a884
Backport overflow checking of `lcm`/`gcd` to C++17
frederick-vs-ja Jul 4, 2024
2c5a2c4
Merge pull request #6 from frederick-vs-ja/overflow-17
AlexGuteniev Jul 4, 2024
c71cd4d
All modes `in_range`
AlexGuteniev Jul 4, 2024
16bd45a
if constexpr untrue
AlexGuteniev Jul 4, 2024
4bc5433
Space. The final frontier.
StephanTLavavej Jul 9, 2024
fe15ffa
Fix arrow.
StephanTLavavej Jul 9, 2024
eed95e6
Extract `_Min_common`.
StephanTLavavej Jul 9, 2024
f83cc82
Early return.
StephanTLavavej Jul 9, 2024
1093d23
Fix gcd/lcm to work with all nonbool integrals.
StephanTLavavej Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion stl/inc/numeric
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,18 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n
using _Common = common_type_t<_Mt, _Nt>;
using _Common_unsigned = make_unsigned_t<_Common>;

if constexpr (is_signed_v<_Common>) {
#ifndef _DEBUG
if (_STD _Is_constant_evaluated())
#endif // ^^^ !defined(_DEBUG) ^^
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
{
if (_Mx == _STD _Min_limit<_Common>() || _Nx == _STD _Min_limit<_Common>()) {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_STL_REPORT_ERROR("Preconditions: |m| and |n| are representable as a value of common_type_t<M, N>."
"(N4981 [numeric.ops.gcd]/2, N4981 [numeric.ops.lcm]/2)");
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

return _Select_countr_zero_impl<_Common_unsigned>([=](auto _Countr_zero_impl) {
_Common_unsigned _Mx_magnitude = _Abs_u(_Mx);
_Common_unsigned _Nx_magnitude = _Abs_u(_Nx);
Expand Down Expand Up @@ -670,7 +682,22 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> lcm(const _Mt _Mx, const _Nt _Nx) n
return 0;
}

return static_cast<_Common>((_Mx_magnitude / _STD gcd(_Mx_magnitude, _Nx_magnitude)) * _Nx_magnitude);
#ifndef _DEBUG
if (!_STD _Is_constant_evaluated()) {
return static_cast<_Common>((_Mx_magnitude / _STD gcd(_Mx_magnitude, _Nx_magnitude)) * _Nx_magnitude);
} else
#endif // ^^^ !defined(_DEBUG) ^^^
{
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_Common_unsigned _Result = 0;
_Common_unsigned _Tmp = _Mx_magnitude / _STD gcd(_Mx_magnitude, _Nx_magnitude);

if (_Mul_overflow(_Tmp, _Nx_magnitude, _Result) || !_In_range<_Common>(_Result)) {
_STL_REPORT_ERROR("Preconditions: The least common multiple of |m| and |n| is representable as a value of "
"type common_type_t<M, N>. (N4981 [numeric.ops.lcm]/2)");
}

return static_cast<_Common>(_Result);
}
}
#endif // _HAS_CXX17

Expand Down
16 changes: 13 additions & 3 deletions stl/inc/xutility
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7372,16 +7372,25 @@ _NODISCARD constexpr bool _Add_overflow(const _Int _Left, const _Int _Right, _In
}
}
}
#endif // _HAS_CXX23

#if _HAS_CXX17
#if _HAS_CXX20
template <_Integer_like _Int>
#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
template <class _Int, enable_if_t<_Is_nonbool_integral<_Int>, int> = 0>
#endif // ^^^ !_HAS_CXX20 ^^^
_NODISCARD constexpr bool _Mul_overflow(const _Int _Left, const _Int _Right, _Int& _Out) {
#if defined(__clang__) && !_HAS_CXX20
return __builtin_mul_overflow(_Left, _Right, &_Out);
#else // ^^^ defined(__clang__) && !_HAS_CXX20 / !defined(__clang__) || _HAS_CXX20 vvv
#ifdef __clang__
if constexpr (integral<_Int>) {
if constexpr (is_integral_v<_Int>) {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
return __builtin_mul_overflow(_Left, _Right, &_Out);
} else
#endif // defined(__clang__)
{
if constexpr (!_Signed_integer_like<_Int>) {
if constexpr (static_cast<_Int>(-1) > static_cast<_Int>(0)) {
constexpr auto _UInt_max = _STD _Max_limit<_Int>();
const bool _Overflow = _Left != 0 && _Right > _UInt_max / _Left;
if (!_Overflow) {
Expand Down Expand Up @@ -7420,8 +7429,9 @@ _NODISCARD constexpr bool _Mul_overflow(const _Int _Left, const _Int _Right, _In
// ^^^ Based on llvm::MulOverflow ^^^
}
}
#endif // ^^^ !defined(__clang__) || _HAS_CXX20 ^^^
}
#endif // _HAS_CXX23
#endif // _HAS_CXX17

_STD_END

Expand Down
2 changes: 1 addition & 1 deletion tests/std/tests/P0295R0_gcd_lcm/test.compile.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ static_assert(gcd(1073741824, 536870912) == 536870912);
static_assert(gcd(1073741824, -536870912) == 536870912);
static_assert(gcd(-1073741824, 536870912) == 536870912);
static_assert(gcd(int_max, int_max) == int_max);
static_assert(gcd(int_min, int_max) == 1);
// gcd(int_min, int_max) -> undefined behavior
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
// gcd(int_min, int_min) -> undefined behavior
static_assert(gcd(int_min + 1, int_min + 1) == int_max);

Expand Down