From 3cee76928fbbfaa6f5cd1ad2a63729eecba3d1e8 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Mon, 1 Jul 2024 15:10:15 +0300 Subject: [PATCH 01/13] ``: check for gcd / lcm overflows In compile time or in debug. Applies to C++20 and later mode. --- stl/inc/numeric | 42 +++++++++++++++++++++++++++++++++++++----- stl/inc/xutility | 4 +++- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index 34287fa0a97..dd37d586842 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -625,15 +625,15 @@ _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>; - return _Select_countr_zero_impl<_Common_unsigned>([=](auto _Countr_zero_impl) { + _Common_unsigned _Result = _Select_countr_zero_impl<_Common_unsigned>([=](auto _Countr_zero_impl) { _Common_unsigned _Mx_magnitude = _Abs_u(_Mx); _Common_unsigned _Nx_magnitude = _Abs_u(_Nx); if (_Mx_magnitude == 0U) { - return static_cast<_Common>(_Nx_magnitude); + return _Nx_magnitude; } if (_Nx_magnitude == 0U) { - return static_cast<_Common>(_Mx_magnitude); + return _Mx_magnitude; } const auto _Mx_trailing_zeroes = static_cast(_Countr_zero_impl(_Mx_magnitude)); @@ -650,12 +650,25 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n _Nx_magnitude -= _Mx_magnitude; if (_Nx_magnitude == 0U) { - return static_cast<_Common>(_Mx_magnitude << _Common_factors_of_2); + return static_cast<_Common_unsigned>(_Mx_magnitude << _Common_factors_of_2); } _Nx_trailing_zeroes = static_cast(_Countr_zero_impl(_Nx_magnitude)); } }); + +#if _HAS_CXX20 +#ifndef _DEBUG + if (_STD _Is_constant_evaluated()) +#endif // ^^^ !defined(_DEBUG) ^^ + { + if (!in_range<_Common>(_Result)) { + _STL_REPORT_ERROR("gcd overflow"); + } + } +#endif // _HAS_CXX20 + + return static_cast<_Common>(_Result); } _EXPORT_STD template @@ -670,7 +683,26 @@ _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); +#if _HAS_CXX20 +#ifdef _DEBUG + if constexpr (true) +#else // ^^^ defined(_DEBUG) / !defined(_DEBUG) vvv + if (_STD _Is_constant_evaluated()) +#endif // ^^^ !defined(_DEBUG) ^^^ + { + _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("lcm overflow"); + } + + return static_cast<_Common>(_Result); + } else +#endif // _HAS_CXX20 + { + return static_cast<_Common>((_Mx_magnitude / _STD gcd(_Mx_magnitude, _Nx_magnitude)) * _Nx_magnitude); + } } #endif // _HAS_CXX17 diff --git a/stl/inc/xutility b/stl/inc/xutility index 988196930f3..eb644e877fc 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -7372,7 +7372,9 @@ _NODISCARD constexpr bool _Add_overflow(const _Int _Left, const _Int _Right, _In } } } +#endif // _HAS_CXX23 +#if _HAS_CXX20 template <_Integer_like _Int> _NODISCARD constexpr bool _Mul_overflow(const _Int _Left, const _Int _Right, _Int& _Out) { #ifdef __clang__ @@ -7421,7 +7423,7 @@ _NODISCARD constexpr bool _Mul_overflow(const _Int _Left, const _Int _Right, _In } } } -#endif // _HAS_CXX23 +#endif // _HAS_CXX20 _STD_END From 7d4bc4d6f93f165bb302a3b6474795aa737dd6d9 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Mon, 1 Jul 2024 18:39:55 +0300 Subject: [PATCH 02/13] formal --- stl/inc/numeric | 51 +++++++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index dd37d586842..a5157592571 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -622,18 +622,43 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n // calculate greatest common divisor static_assert(_Is_nonbool_integral<_Mt> && _Is_nonbool_integral<_Nt>, "GCD requires nonbool integral types"); +#if _HAS_CXX20 + if constexpr (is_signed_v<_Mt> || is_signed_v<_Nt>) { +#ifndef _DEBUG + if (_STD is_constant_evaluated()) +#endif // ^^^ !defined(_DEBUG) ^^ + { + if constexpr (is_signed_v<_Mt>) { + if (_Mx == _STD _Min_limit<_Mt>()) { + _STL_REPORT_ERROR( + "Preconditions: |m| and |n| are representable as a value of common_type_t." + "(N4981 [numeric.ops.gcd]/2, N4981 [numeric.ops.lcm]/2)")); + } + } + + if constexpr (is_signed_v<_Nt>) { + if (_Nx == _STD _Min_limit<_Mt>()) { + _STL_REPORT_ERROR( + "Preconditions: |m| and |n| are representable as a value of common_type_t." + "(N4981 [numeric.ops.gcd]/2, N4981 [numeric.ops.lcm]/2)")); + } + } + } + } +#endif // _HAS_CXX20 + using _Common = common_type_t<_Mt, _Nt>; using _Common_unsigned = make_unsigned_t<_Common>; - _Common_unsigned _Result = _Select_countr_zero_impl<_Common_unsigned>([=](auto _Countr_zero_impl) { + 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); if (_Mx_magnitude == 0U) { - return _Nx_magnitude; + return static_cast<_Common>(_Nx_magnitude); } if (_Nx_magnitude == 0U) { - return _Mx_magnitude; + return static_cast<_Common>(_Mx_magnitude); } const auto _Mx_trailing_zeroes = static_cast(_Countr_zero_impl(_Mx_magnitude)); @@ -650,25 +675,12 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n _Nx_magnitude -= _Mx_magnitude; if (_Nx_magnitude == 0U) { - return static_cast<_Common_unsigned>(_Mx_magnitude << _Common_factors_of_2); + return static_cast<_Common>(_Mx_magnitude << _Common_factors_of_2); } _Nx_trailing_zeroes = static_cast(_Countr_zero_impl(_Nx_magnitude)); } }); - -#if _HAS_CXX20 -#ifndef _DEBUG - if (_STD _Is_constant_evaluated()) -#endif // ^^^ !defined(_DEBUG) ^^ - { - if (!in_range<_Common>(_Result)) { - _STL_REPORT_ERROR("gcd overflow"); - } - } -#endif // _HAS_CXX20 - - return static_cast<_Common>(_Result); } _EXPORT_STD template @@ -687,14 +699,15 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> lcm(const _Mt _Mx, const _Nt _Nx) n #ifdef _DEBUG if constexpr (true) #else // ^^^ defined(_DEBUG) / !defined(_DEBUG) vvv - if (_STD _Is_constant_evaluated()) + if (_STD is_constant_evaluated()) #endif // ^^^ !defined(_DEBUG) ^^^ { _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("lcm overflow"); + _STL_REPORT_ERROR("Preconditions: The least common multiple of |m| and |n| is representable as a value of " + "type common_type_t. (N4981 [numeric.ops.lcm]/2)"); } return static_cast<_Common>(_Result); From d9c0c8dda092bc7b1539459e28dd4767b97eab26 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Mon, 1 Jul 2024 18:40:55 +0300 Subject: [PATCH 03/13] `_Nt` --- stl/inc/numeric | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index a5157592571..3e6a993d878 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -637,7 +637,7 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n } if constexpr (is_signed_v<_Nt>) { - if (_Nx == _STD _Min_limit<_Mt>()) { + if (_Nx == _STD _Min_limit<_Nt>()) { _STL_REPORT_ERROR( "Preconditions: |m| and |n| are representable as a value of common_type_t." "(N4981 [numeric.ops.gcd]/2, N4981 [numeric.ops.lcm]/2)")); From 563350646729ffd8c34e9794282d1327dbfba4e6 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Mon, 1 Jul 2024 18:49:38 +0300 Subject: [PATCH 04/13] check against common type --- stl/inc/numeric | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index 3e6a993d878..b0429b54f95 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -622,34 +622,23 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n // calculate greatest common divisor static_assert(_Is_nonbool_integral<_Mt> && _Is_nonbool_integral<_Nt>, "GCD requires nonbool integral types"); + using _Common = common_type_t<_Mt, _Nt>; + using _Common_unsigned = make_unsigned_t<_Common>; + #if _HAS_CXX20 - if constexpr (is_signed_v<_Mt> || is_signed_v<_Nt>) { + if constexpr (is_signed_v<_Common>) { #ifndef _DEBUG if (_STD is_constant_evaluated()) #endif // ^^^ !defined(_DEBUG) ^^ { - if constexpr (is_signed_v<_Mt>) { - if (_Mx == _STD _Min_limit<_Mt>()) { - _STL_REPORT_ERROR( - "Preconditions: |m| and |n| are representable as a value of common_type_t." - "(N4981 [numeric.ops.gcd]/2, N4981 [numeric.ops.lcm]/2)")); - } - } - - if constexpr (is_signed_v<_Nt>) { - if (_Nx == _STD _Min_limit<_Nt>()) { - _STL_REPORT_ERROR( - "Preconditions: |m| and |n| are representable as a value of common_type_t." - "(N4981 [numeric.ops.gcd]/2, N4981 [numeric.ops.lcm]/2)")); - } + if (_Mx == _STD _Min_limit<_Common>() && _Nx == _STD _Min_limit<_Common>()) { + _STL_REPORT_ERROR("Preconditions: |m| and |n| are representable as a value of common_type_t." + "(N4981 [numeric.ops.gcd]/2, N4981 [numeric.ops.lcm]/2)"); } } } #endif // _HAS_CXX20 - using _Common = common_type_t<_Mt, _Nt>; - using _Common_unsigned = make_unsigned_t<_Common>; - 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); From 8c95ecd5cfc65eadfa8c99a3f60861497b0fcf1b Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Mon, 1 Jul 2024 18:53:33 +0300 Subject: [PATCH 05/13] actually this was a bogus test --- stl/inc/numeric | 2 +- tests/std/tests/P0295R0_gcd_lcm/test.compile.pass.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index b0429b54f95..ce5e131b112 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -631,7 +631,7 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n if (_STD is_constant_evaluated()) #endif // ^^^ !defined(_DEBUG) ^^ { - if (_Mx == _STD _Min_limit<_Common>() && _Nx == _STD _Min_limit<_Common>()) { + if (_Mx == _STD _Min_limit<_Common>() || _Nx == _STD _Min_limit<_Common>()) { _STL_REPORT_ERROR("Preconditions: |m| and |n| are representable as a value of common_type_t." "(N4981 [numeric.ops.gcd]/2, N4981 [numeric.ops.lcm]/2)"); } diff --git a/tests/std/tests/P0295R0_gcd_lcm/test.compile.pass.cpp b/tests/std/tests/P0295R0_gcd_lcm/test.compile.pass.cpp index 03f56a523dd..55f63d88ca8 100644 --- a/tests/std/tests/P0295R0_gcd_lcm/test.compile.pass.cpp +++ b/tests/std/tests/P0295R0_gcd_lcm/test.compile.pass.cpp @@ -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 // gcd(int_min, int_min) -> undefined behavior static_assert(gcd(int_min + 1, int_min + 1) == int_max); From 260a884ee62df7fd3b6b661baf64b99eb516ef29 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Thu, 4 Jul 2024 16:30:53 +0800 Subject: [PATCH 06/13] Backport overflow checking of `lcm`/`gcd` to C++17 `_Mul_overflow` is made to only handle non-`bool` integer types in C++17. --- stl/inc/numeric | 11 +++-------- stl/inc/xutility | 14 +++++++++++--- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index ce5e131b112..0dafdfa9e3e 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -625,10 +625,9 @@ _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 _HAS_CXX20 if constexpr (is_signed_v<_Common>) { #ifndef _DEBUG - if (_STD is_constant_evaluated()) + if (_STD _Is_constant_evaluated()) #endif // ^^^ !defined(_DEBUG) ^^ { if (_Mx == _STD _Min_limit<_Common>() || _Nx == _STD _Min_limit<_Common>()) { @@ -637,7 +636,6 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n } } } -#endif // _HAS_CXX20 return _Select_countr_zero_impl<_Common_unsigned>([=](auto _Countr_zero_impl) { _Common_unsigned _Mx_magnitude = _Abs_u(_Mx); @@ -684,11 +682,10 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> lcm(const _Mt _Mx, const _Nt _Nx) n return 0; } -#if _HAS_CXX20 #ifdef _DEBUG if constexpr (true) #else // ^^^ defined(_DEBUG) / !defined(_DEBUG) vvv - if (_STD is_constant_evaluated()) + if (_STD _Is_constant_evaluated()) #endif // ^^^ !defined(_DEBUG) ^^^ { _Common_unsigned _Result = 0; @@ -700,9 +697,7 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> lcm(const _Mt _Mx, const _Nt _Nx) n } return static_cast<_Common>(_Result); - } else -#endif // _HAS_CXX20 - { + } else { return static_cast<_Common>((_Mx_magnitude / _STD gcd(_Mx_magnitude, _Nx_magnitude)) * _Nx_magnitude); } } diff --git a/stl/inc/xutility b/stl/inc/xutility index eb644e877fc..2beb7c6dd2e 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -7374,16 +7374,23 @@ _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 , 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>) { 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) { @@ -7422,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_CXX20 +#endif // _HAS_CXX17 _STD_END From c71cd4d892dcb819d43c16f5e221c7df5518799b Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 4 Jul 2024 12:06:33 +0300 Subject: [PATCH 07/13] All modes `in_range` --- stl/inc/numeric | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index 0dafdfa9e3e..94e9e5eb7ec 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -691,7 +691,7 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> lcm(const _Mt _Mx, const _Nt _Nx) n _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)) { + 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. (N4981 [numeric.ops.lcm]/2)"); } From 16bd45a1fb910b2e0c895aad16a4ab7b92f63c9e Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 4 Jul 2024 12:09:04 +0300 Subject: [PATCH 08/13] if constexpr untrue --- stl/inc/numeric | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index 94e9e5eb7ec..ca80fb5f39f 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -682,10 +682,10 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> lcm(const _Mt _Mx, const _Nt _Nx) n return 0; } -#ifdef _DEBUG - if constexpr (true) -#else // ^^^ defined(_DEBUG) / !defined(_DEBUG) vvv - if (_STD _Is_constant_evaluated()) +#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) ^^^ { _Common_unsigned _Result = 0; @@ -697,8 +697,6 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> lcm(const _Mt _Mx, const _Nt _Nx) n } return static_cast<_Common>(_Result); - } else { - return static_cast<_Common>((_Mx_magnitude / _STD gcd(_Mx_magnitude, _Nx_magnitude)) * _Nx_magnitude); } } #endif // _HAS_CXX17 From 4bc543385cc436760e5278b319f84104e6cae138 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 8 Jul 2024 18:01:46 -0700 Subject: [PATCH 09/13] Space. The final frontier. --- stl/inc/numeric | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index ca80fb5f39f..406ea917557 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -631,7 +631,7 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n #endif // ^^^ !defined(_DEBUG) ^^ { if (_Mx == _STD _Min_limit<_Common>() || _Nx == _STD _Min_limit<_Common>()) { - _STL_REPORT_ERROR("Preconditions: |m| and |n| are representable as a value of common_type_t." + _STL_REPORT_ERROR("Preconditions: |m| and |n| are representable as a value of common_type_t. " "(N4981 [numeric.ops.gcd]/2, N4981 [numeric.ops.lcm]/2)"); } } From fe15ffae347aea13ff3f36cf539219ddaa353514 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 8 Jul 2024 18:10:42 -0700 Subject: [PATCH 10/13] Fix arrow. --- stl/inc/numeric | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index 406ea917557..30fa2e22049 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -628,7 +628,7 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n if constexpr (is_signed_v<_Common>) { #ifndef _DEBUG if (_STD _Is_constant_evaluated()) -#endif // ^^^ !defined(_DEBUG) ^^ +#endif // ^^^ !defined(_DEBUG) ^^^ { if (_Mx == _STD _Min_limit<_Common>() || _Nx == _STD _Min_limit<_Common>()) { _STL_REPORT_ERROR("Preconditions: |m| and |n| are representable as a value of common_type_t. " From eed95e64697059f83f361e3b4e5af7ac7a2dbdfa Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 8 Jul 2024 18:13:05 -0700 Subject: [PATCH 11/13] Extract `_Min_common`. --- stl/inc/numeric | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index 30fa2e22049..130b3f02ad5 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -630,7 +630,8 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> gcd(const _Mt _Mx, const _Nt _Nx) n if (_STD _Is_constant_evaluated()) #endif // ^^^ !defined(_DEBUG) ^^^ { - if (_Mx == _STD _Min_limit<_Common>() || _Nx == _STD _Min_limit<_Common>()) { + constexpr auto _Min_common = _STD _Min_limit<_Common>(); + if (_Mx == _Min_common || _Nx == _Min_common) { _STL_REPORT_ERROR("Preconditions: |m| and |n| are representable as a value of common_type_t. " "(N4981 [numeric.ops.gcd]/2, N4981 [numeric.ops.lcm]/2)"); } From f83cc8270e382372e87282cf4a271e73c284ff6f Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 8 Jul 2024 18:19:40 -0700 Subject: [PATCH 12/13] Early return. --- stl/inc/numeric | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index 130b3f02ad5..7547a7330d4 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -686,19 +686,18 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> lcm(const _Mt _Mx, const _Nt _Nx) n #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) ^^^ - { - _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. (N4981 [numeric.ops.lcm]/2)"); - } + _Common_unsigned _Result = 0; + _Common_unsigned _Tmp = _Mx_magnitude / _STD gcd(_Mx_magnitude, _Nx_magnitude); - return static_cast<_Common>(_Result); + 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. (N4981 [numeric.ops.lcm]/2)"); } + + return static_cast<_Common>(_Result); } #endif // _HAS_CXX17 From 1093d23e5a812454ee62673884aafed3da769073 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 10 Jul 2024 22:00:14 -0700 Subject: [PATCH 13/13] Fix gcd/lcm to work with all nonbool integrals. numeric: Add a `static_cast` when dividing `_Common_unsigned` by `_Common_unsigned`, due to the usual arithmetic conversions which will emit sign conversion warnings for tiny types. utility: * Move `_Is_standard_integer` down to the `_HAS_CXX20` region. (It's also used by ``.) * Relax the internal `_Cmp_equal`, `_Cmp_less`, and `_In_range` to accept nonbool integrals. + Because they're internal, we can `_STL_INTERNAL_STATIC_ASSERT`. + Comment that this "allows character types". * Enforce `_Is_standard_integer` at the user-visible layer. P0295R0_gcd_lcm: Add test coverage, because gcd/lcm are required to accept nonbool integrals. (An MSVC-internal test found this by using `char`.) --- stl/inc/numeric | 2 +- stl/inc/utility | 31 +++++++++++++------ .../P0295R0_gcd_lcm/test.compile.pass.cpp | 25 +++++++++++++++ 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/stl/inc/numeric b/stl/inc/numeric index 7547a7330d4..528969fdc63 100644 --- a/stl/inc/numeric +++ b/stl/inc/numeric @@ -690,7 +690,7 @@ _NODISCARD constexpr common_type_t<_Mt, _Nt> lcm(const _Mt _Mx, const _Nt _Nx) n #endif // ^^^ !defined(_DEBUG) ^^^ _Common_unsigned _Result = 0; - _Common_unsigned _Tmp = _Mx_magnitude / _STD gcd(_Mx_magnitude, _Nx_magnitude); + _Common_unsigned _Tmp = static_cast<_Common_unsigned>(_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 " diff --git a/stl/inc/utility b/stl/inc/utility index d5e58acc007..2eb8ada2d52 100644 --- a/stl/inc/utility +++ b/stl/inc/utility @@ -784,14 +784,9 @@ _EXPORT_STD template constexpr in_place_index_t<_Idx> in_place_index{}; #endif // _HAS_CXX17 -template -constexpr bool _Is_standard_integer = _Is_any_of_v, signed char, short, int, long, long long, - unsigned char, unsigned short, unsigned int, unsigned long, unsigned long long>; - template _NODISCARD constexpr bool _Cmp_equal(const _Ty1 _Left, const _Ty2 _Right) noexcept { - static_assert(_Is_standard_integer<_Ty1> && _Is_standard_integer<_Ty2>, - "The integer comparison functions only accept standard and extended integer types."); + _STL_INTERNAL_STATIC_ASSERT(_Is_nonbool_integral<_Ty1> && _Is_nonbool_integral<_Ty2>); // allows character types if constexpr (is_signed_v<_Ty1> == is_signed_v<_Ty2>) { return _Left == _Right; } else if constexpr (is_signed_v<_Ty2>) { @@ -808,8 +803,7 @@ _NODISCARD constexpr bool _Cmp_not_equal(const _Ty1 _Left, const _Ty2 _Right) no template _NODISCARD constexpr bool _Cmp_less(const _Ty1 _Left, const _Ty2 _Right) noexcept { - static_assert(_Is_standard_integer<_Ty1> && _Is_standard_integer<_Ty2>, - "The integer comparison functions only accept standard and extended integer types."); + _STL_INTERNAL_STATIC_ASSERT(_Is_nonbool_integral<_Ty1> && _Is_nonbool_integral<_Ty2>); // allows character types if constexpr (is_signed_v<_Ty1> == is_signed_v<_Ty2>) { return _Left < _Right; } else if constexpr (is_signed_v<_Ty2>) { @@ -858,8 +852,7 @@ _NODISCARD constexpr _Ty _Max_limit() noexcept { // same as (numeric_limits<_Ty> template _NODISCARD constexpr bool _In_range(const _Ty _Value) noexcept { - static_assert(_Is_standard_integer<_Rx> && _Is_standard_integer<_Ty>, - "The integer comparison functions only accept standard and extended integer types."); + _STL_INTERNAL_STATIC_ASSERT(_Is_nonbool_integral<_Rx> && _Is_nonbool_integral<_Ty>); // allows character types constexpr auto _Ty_min = _Min_limit<_Ty>(); constexpr auto _Rx_min = _Min_limit<_Rx>(); @@ -883,38 +876,56 @@ _NODISCARD constexpr bool _In_range(const _Ty _Value) noexcept { } #if _HAS_CXX20 +template +constexpr bool _Is_standard_integer = _Is_any_of_v, signed char, short, int, long, long long, + unsigned char, unsigned short, unsigned int, unsigned long, unsigned long long>; + _EXPORT_STD template _NODISCARD constexpr bool cmp_equal(const _Ty1 _Left, const _Ty2 _Right) noexcept { + static_assert(_Is_standard_integer<_Ty1> && _Is_standard_integer<_Ty2>, + "The integer comparison functions only accept standard and extended integer types."); return _STD _Cmp_equal(_Left, _Right); } _EXPORT_STD template _NODISCARD constexpr bool cmp_not_equal(const _Ty1 _Left, const _Ty2 _Right) noexcept { + static_assert(_Is_standard_integer<_Ty1> && _Is_standard_integer<_Ty2>, + "The integer comparison functions only accept standard and extended integer types."); return _STD _Cmp_not_equal(_Left, _Right); } _EXPORT_STD template _NODISCARD constexpr bool cmp_less(const _Ty1 _Left, const _Ty2 _Right) noexcept { + static_assert(_Is_standard_integer<_Ty1> && _Is_standard_integer<_Ty2>, + "The integer comparison functions only accept standard and extended integer types."); return _STD _Cmp_less(_Left, _Right); } _EXPORT_STD template _NODISCARD constexpr bool cmp_greater(const _Ty1 _Left, const _Ty2 _Right) noexcept { + static_assert(_Is_standard_integer<_Ty1> && _Is_standard_integer<_Ty2>, + "The integer comparison functions only accept standard and extended integer types."); return _STD _Cmp_greater(_Left, _Right); } _EXPORT_STD template _NODISCARD constexpr bool cmp_less_equal(const _Ty1 _Left, const _Ty2 _Right) noexcept { + static_assert(_Is_standard_integer<_Ty1> && _Is_standard_integer<_Ty2>, + "The integer comparison functions only accept standard and extended integer types."); return _STD _Cmp_less_equal(_Left, _Right); } _EXPORT_STD template _NODISCARD constexpr bool cmp_greater_equal(const _Ty1 _Left, const _Ty2 _Right) noexcept { + static_assert(_Is_standard_integer<_Ty1> && _Is_standard_integer<_Ty2>, + "The integer comparison functions only accept standard and extended integer types."); return _STD _Cmp_greater_equal(_Left, _Right); } _EXPORT_STD template _NODISCARD constexpr bool in_range(const _Ty _Value) noexcept { + static_assert(_Is_standard_integer<_Rx> && _Is_standard_integer<_Ty>, + "The integer comparison functions only accept standard and extended integer types."); return _STD _In_range<_Rx>(_Value); } #endif // _HAS_CXX20 diff --git a/tests/std/tests/P0295R0_gcd_lcm/test.compile.pass.cpp b/tests/std/tests/P0295R0_gcd_lcm/test.compile.pass.cpp index 55f63d88ca8..58ac9095bfc 100644 --- a/tests/std/tests/P0295R0_gcd_lcm/test.compile.pass.cpp +++ b/tests/std/tests/P0295R0_gcd_lcm/test.compile.pass.cpp @@ -49,3 +49,28 @@ static_assert(lcm(1, 0) == 0); static_assert(lcm(1073741824, 536870912) == 1073741824); static_assert(lcm(1073741824, -536870912) == 1073741824); static_assert(lcm(-1073741824, 536870912) == 1073741824); + +template +constexpr bool test_nonbool_integral_type() { + static_assert(gcd(T{60}, T{24}) == T{12}); + static_assert(lcm(T{60}, T{24}) == T{120}); + return true; +} + +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +#ifdef __cpp_char8_t +static_assert(test_nonbool_integral_type()); +#endif // __cpp_char8_t +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type()); +static_assert(test_nonbool_integral_type());