From aff72b0997e1e211d76923b60b4eac33b9c7a1cd Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 29 Apr 2025 13:58:46 +0800 Subject: [PATCH 1/4] Make algorithms properly destroy objects in constant evaluation --- stl/inc/memory | 41 +++++++++++++++---- stl/inc/xmemory | 12 +++++- .../test.cpp | 36 ++++++++++++++++ 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/stl/inc/memory b/stl/inc/memory index 8791b476763..f7a440fdaa9 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -584,12 +584,18 @@ namespace ranges { template <_No_throw_input_iterator _It, _No_throw_sentinel_for<_It> _Se> requires destructible> _NODISCARD constexpr _It _Destroy_unchecked(_It _First, _Se _Last) noexcept { - if constexpr (is_trivially_destructible_v>) { - _RANGES advance(_First, _STD move(_Last)); - } else { + if consteval { for (; _First != _Last; ++_First) { _RANGES destroy_at(_STD addressof(*_First)); } + } else { + if constexpr (is_trivially_destructible_v>) { + _RANGES advance(_First, _STD move(_Last)); + } else { + for (; _First != _Last; ++_First) { + _RANGES destroy_at(_STD addressof(*_First)); + } + } } return _First; @@ -630,12 +636,21 @@ _CONSTEXPR20 _NoThrowFwdIt destroy_n(_NoThrowFwdIt _First, const _Diff _Count_ra } auto _UFirst = _STD _Get_unwrapped_n(_First, _Count); - if constexpr (is_trivially_destructible_v<_Iter_value_t<_NoThrowFwdIt>>) { - _STD advance(_UFirst, _Count); - } else { +#if _HAS_CXX20 + if consteval { for (; _Count > 0; --_Count, (void) ++_UFirst) { _STD _Destroy_in_place(*_UFirst); } + } else +#endif // _HAS_CXX20 + { + if constexpr (is_trivially_destructible_v<_Iter_value_t<_NoThrowFwdIt>>) { + _STD advance(_UFirst, _Count); + } else { + for (; _Count > 0; --_Count, (void) ++_UFirst) { + _STD _Destroy_in_place(*_UFirst); + } + } } _STD _Seek_wrapped(_First, _UFirst); @@ -658,14 +673,22 @@ namespace ranges { } auto _UFirst = _STD _Get_unwrapped_n(_STD move(_First), _Count); - if constexpr (is_trivially_destructible_v>) { - _RANGES advance(_UFirst, _Count); - } else { + if consteval { do { _RANGES destroy_at(_STD addressof(*_UFirst)); ++_UFirst; --_Count; } while (_Count > 0); + } else { + if constexpr (is_trivially_destructible_v>) { + _RANGES advance(_UFirst, _Count); + } else { + do { + _RANGES destroy_at(_STD addressof(*_UFirst)); + ++_UFirst; + --_Count; + } while (_Count > 0); + } } _STD _Seek_wrapped(_First, _STD move(_UFirst)); diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 80041fc71bc..1e43553fb18 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1111,11 +1111,19 @@ _CONSTEXPR20 void _Destroy_range(_Alloc_ptr_t<_Alloc> _First, const _Alloc_ptr_t template _CONSTEXPR20 void _Destroy_range(_NoThrowFwdIt _First, const _NoThrowSentinel _Last) noexcept { - // note that this is an optimization for debug mode codegen; in release mode the BE removes all of this - if constexpr (!is_trivially_destructible_v<_Iter_value_t<_NoThrowFwdIt>>) { +#if _HAS_CXX20 + if consteval { for (; _First != _Last; ++_First) { _STD _Destroy_in_place(*_First); } + } else +#endif // _HAS_CXX20 + { // note that this is an optimization for debug mode codegen; in release mode the BE removes all of this + if constexpr (!is_trivially_destructible_v<_Iter_value_t<_NoThrowFwdIt>>) { + for (; _First != _Last; ++_First) { + _STD _Destroy_in_place(*_First); + } + } } } diff --git a/tests/std/tests/P0784R7_library_support_for_more_constexpr_containers/test.cpp b/tests/std/tests/P0784R7_library_support_for_more_constexpr_containers/test.cpp index 3fdcdca2b3e..bc7ba1de7ce 100644 --- a/tests/std/tests/P0784R7_library_support_for_more_constexpr_containers/test.cpp +++ b/tests/std/tests/P0784R7_library_support_for_more_constexpr_containers/test.cpp @@ -583,6 +583,42 @@ static_assert(!CanDestroyN); static_assert(!CanDestroyN); static_assert(!CanDestroyN); +#if defined(__clang__) && !defined(__EDG__) // TRANSITION, DevCom-10642767 (MSVC), DevCom-10896316 (EDG) +// Test that destroy, destroy_at, and destroy_n properly destroy trivially destructible objects +// during constant evaluation. +// After such destruction, further access will cause core language undefined behavior, +// which will in turn cause constant evaluation failure. + +template +struct require_valid_constant; + +template +constexpr int consteval_validate_destruction(Fn op) { + struct S { + int n; + }; + + S arr[1]{{42}}; + op(arr); + return arr[0].n; +} + +template +constexpr bool CanWellDefinedlyAccessAfterOperation = + requires { typename require_valid_constant; }; + +static_assert(CanWellDefinedlyAccessAfterOperation<[](auto&) {}>); +static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { destroy(arr + 0, arr + 1); }>); +static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { destroy_at(arr + 0); }>); +static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { destroy_at(&arr); }>); +static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { destroy_n(arr + 0, 1); }>); +static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { ranges::destroy(arr + 0, arr + 1); }>); +static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { ranges::destroy(arr); }>); +static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { ranges::destroy_at(arr + 0); }>); +static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { ranges::destroy_at(&arr); }>); +static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { ranges::destroy_n(arr + 0, 1); }>); +#endif // defined(__clang__) && !defined(__EDG__) + int main() { test_runtime(1234); test_runtime(string("hello world")); From 734b79a543142535faef8a95a356d588058bd217 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 29 Apr 2025 14:52:09 +0800 Subject: [PATCH 2/4] Suppress warnings for MSVC, workaround for EDG etc. --- stl/inc/memory | 6 +++--- stl/inc/xmemory | 2 +- stl/inc/yvals_core.h | 28 ++++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/stl/inc/memory b/stl/inc/memory index f7a440fdaa9..26916ceda3b 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -584,7 +584,7 @@ namespace ranges { template <_No_throw_input_iterator _It, _No_throw_sentinel_for<_It> _Se> requires destructible> _NODISCARD constexpr _It _Destroy_unchecked(_It _First, _Se _Last) noexcept { - if consteval { + if _STL_CONSTEVAL_CONDITION { for (; _First != _Last; ++_First) { _RANGES destroy_at(_STD addressof(*_First)); } @@ -637,7 +637,7 @@ _CONSTEXPR20 _NoThrowFwdIt destroy_n(_NoThrowFwdIt _First, const _Diff _Count_ra auto _UFirst = _STD _Get_unwrapped_n(_First, _Count); #if _HAS_CXX20 - if consteval { + if _STL_CONSTEVAL_CONDITION { for (; _Count > 0; --_Count, (void) ++_UFirst) { _STD _Destroy_in_place(*_UFirst); } @@ -673,7 +673,7 @@ namespace ranges { } auto _UFirst = _STD _Get_unwrapped_n(_STD move(_First), _Count); - if consteval { + if _STL_CONSTEVAL_CONDITION { do { _RANGES destroy_at(_STD addressof(*_UFirst)); ++_UFirst; diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 1e43553fb18..6daaa3b3688 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1112,7 +1112,7 @@ _CONSTEXPR20 void _Destroy_range(_Alloc_ptr_t<_Alloc> _First, const _Alloc_ptr_t template _CONSTEXPR20 void _Destroy_range(_NoThrowFwdIt _First, const _NoThrowSentinel _Last) noexcept { #if _HAS_CXX20 - if consteval { + if _STL_CONSTEVAL_CONDITION { for (; _First != _Last; ++_First) { _STD _Destroy_in_place(*_First); } diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index fe1db005f75..e9c18510133 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -765,6 +765,13 @@ #define _STL_DISABLED_WARNING_C4984 #endif +// warning C5282: 'if consteval' requires at least '/std:c++23preview' +#if !_HAS_CXX23 +#define _STL_DISABLED_WARNING_C5282 5282 +#else +#define _STL_DISABLED_WARNING_C5282 +#endif + // warning C5053: support for 'explicit()' in C++17 and earlier is a vendor extension #if !_HAS_CXX20 #define _STL_DISABLED_WARNING_C5053 5053 @@ -820,6 +827,7 @@ _STL_DISABLED_WARNING_C4577 \ _STL_DISABLED_WARNING_C4984 \ _STL_DISABLED_WARNING_C5053 \ + _STL_DISABLED_WARNING_C5282 \ _STL_EXTRA_DISABLED_WARNINGS // clang-format on #endif // !defined(_STL_DISABLED_WARNINGS) @@ -828,6 +836,7 @@ // warning: explicit(bool) is a C++20 extension [-Wc++20-extensions] // warning: declaring overloaded 'operator()' as 'static' is a C++23 extension [-Wc++23-extensions] // warning: static lambdas are a C++23 extension [-Wc++23-extensions] +// warning: consteval if is a C++23 extension [-Wc++23-extensions] // warning: ignoring __declspec(allocator) because the function return type '%s' is not a pointer or reference type // [-Wignored-attributes] // warning: '#pragma float_control' is not supported on this target - ignored [-Wignored-pragmas] @@ -2034,5 +2043,24 @@ compiler option, or define _ALLOW_RTCc_IN_STL to suppress this error. #define _RESTRICT __restrict #endif // ^^^ !defined(__CUDACC__) ^^^ +#if _HAS_CXX20 // see GH-5225 +#if defined(__EDG__) +#define _HAS_IF_CONSTEVAL _HAS_CXX23 +#elif defined(__clang__) || !defined(__CUDACC__) // ^^^ EDG / Clang or MSVC vvv +#define _HAS_IF_CONSTEVAL 1 +#else // ^^^ Clang or MSVC / others vvv +#define _HAS_IF_CONSTEVAL defined(__cpp_if_consteval) +#endif // ^^^ others ^^^ + +#if _HAS_IF_CONSTEVAL +#define _STL_CONSTEVAL_CONDITION consteval +#else // ^^^ no workaround / workaround vvv +#define _STL_CONSTEVAL_CONDITION (_STD is_constant_evaluated()) +#endif // ^^^ workaround ^^^ + +#undef _HAS_IF_CONSTEVAL + +#endif // _HAS_CXX20 + #endif // _STL_COMPILER_PREPROCESSOR #endif // _YVALS_CORE_H_ From 826c5bed0309107199a15c87bace3df9db15f840 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sat, 3 May 2025 10:39:49 +0800 Subject: [PATCH 3/4] Revert "Suppress warnings for MSVC, workaround for EDG etc." This reverts commit 734b79a543142535faef8a95a356d588058bd217. --- stl/inc/memory | 6 +++--- stl/inc/xmemory | 2 +- stl/inc/yvals_core.h | 28 ---------------------------- 3 files changed, 4 insertions(+), 32 deletions(-) diff --git a/stl/inc/memory b/stl/inc/memory index 26916ceda3b..f7a440fdaa9 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -584,7 +584,7 @@ namespace ranges { template <_No_throw_input_iterator _It, _No_throw_sentinel_for<_It> _Se> requires destructible> _NODISCARD constexpr _It _Destroy_unchecked(_It _First, _Se _Last) noexcept { - if _STL_CONSTEVAL_CONDITION { + if consteval { for (; _First != _Last; ++_First) { _RANGES destroy_at(_STD addressof(*_First)); } @@ -637,7 +637,7 @@ _CONSTEXPR20 _NoThrowFwdIt destroy_n(_NoThrowFwdIt _First, const _Diff _Count_ra auto _UFirst = _STD _Get_unwrapped_n(_First, _Count); #if _HAS_CXX20 - if _STL_CONSTEVAL_CONDITION { + if consteval { for (; _Count > 0; --_Count, (void) ++_UFirst) { _STD _Destroy_in_place(*_UFirst); } @@ -673,7 +673,7 @@ namespace ranges { } auto _UFirst = _STD _Get_unwrapped_n(_STD move(_First), _Count); - if _STL_CONSTEVAL_CONDITION { + if consteval { do { _RANGES destroy_at(_STD addressof(*_UFirst)); ++_UFirst; diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 6daaa3b3688..1e43553fb18 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1112,7 +1112,7 @@ _CONSTEXPR20 void _Destroy_range(_Alloc_ptr_t<_Alloc> _First, const _Alloc_ptr_t template _CONSTEXPR20 void _Destroy_range(_NoThrowFwdIt _First, const _NoThrowSentinel _Last) noexcept { #if _HAS_CXX20 - if _STL_CONSTEVAL_CONDITION { + if consteval { for (; _First != _Last; ++_First) { _STD _Destroy_in_place(*_First); } diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index e9c18510133..fe1db005f75 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -765,13 +765,6 @@ #define _STL_DISABLED_WARNING_C4984 #endif -// warning C5282: 'if consteval' requires at least '/std:c++23preview' -#if !_HAS_CXX23 -#define _STL_DISABLED_WARNING_C5282 5282 -#else -#define _STL_DISABLED_WARNING_C5282 -#endif - // warning C5053: support for 'explicit()' in C++17 and earlier is a vendor extension #if !_HAS_CXX20 #define _STL_DISABLED_WARNING_C5053 5053 @@ -827,7 +820,6 @@ _STL_DISABLED_WARNING_C4577 \ _STL_DISABLED_WARNING_C4984 \ _STL_DISABLED_WARNING_C5053 \ - _STL_DISABLED_WARNING_C5282 \ _STL_EXTRA_DISABLED_WARNINGS // clang-format on #endif // !defined(_STL_DISABLED_WARNINGS) @@ -836,7 +828,6 @@ // warning: explicit(bool) is a C++20 extension [-Wc++20-extensions] // warning: declaring overloaded 'operator()' as 'static' is a C++23 extension [-Wc++23-extensions] // warning: static lambdas are a C++23 extension [-Wc++23-extensions] -// warning: consteval if is a C++23 extension [-Wc++23-extensions] // warning: ignoring __declspec(allocator) because the function return type '%s' is not a pointer or reference type // [-Wignored-attributes] // warning: '#pragma float_control' is not supported on this target - ignored [-Wignored-pragmas] @@ -2043,24 +2034,5 @@ compiler option, or define _ALLOW_RTCc_IN_STL to suppress this error. #define _RESTRICT __restrict #endif // ^^^ !defined(__CUDACC__) ^^^ -#if _HAS_CXX20 // see GH-5225 -#if defined(__EDG__) -#define _HAS_IF_CONSTEVAL _HAS_CXX23 -#elif defined(__clang__) || !defined(__CUDACC__) // ^^^ EDG / Clang or MSVC vvv -#define _HAS_IF_CONSTEVAL 1 -#else // ^^^ Clang or MSVC / others vvv -#define _HAS_IF_CONSTEVAL defined(__cpp_if_consteval) -#endif // ^^^ others ^^^ - -#if _HAS_IF_CONSTEVAL -#define _STL_CONSTEVAL_CONDITION consteval -#else // ^^^ no workaround / workaround vvv -#define _STL_CONSTEVAL_CONDITION (_STD is_constant_evaluated()) -#endif // ^^^ workaround ^^^ - -#undef _HAS_IF_CONSTEVAL - -#endif // _HAS_CXX20 - #endif // _STL_COMPILER_PREPROCESSOR #endif // _YVALS_CORE_H_ From 39053a25116648849af9f56d811a03b6c63ef85d Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sat, 3 May 2025 10:45:56 +0800 Subject: [PATCH 4/4] Switch back to `std::is_constant_evaluated`, modify the test. --- stl/inc/memory | 6 +++--- stl/inc/xmemory | 2 +- .../test.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/stl/inc/memory b/stl/inc/memory index f7a440fdaa9..e959623f7b4 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -584,7 +584,7 @@ namespace ranges { template <_No_throw_input_iterator _It, _No_throw_sentinel_for<_It> _Se> requires destructible> _NODISCARD constexpr _It _Destroy_unchecked(_It _First, _Se _Last) noexcept { - if consteval { + if (_STD is_constant_evaluated()) { for (; _First != _Last; ++_First) { _RANGES destroy_at(_STD addressof(*_First)); } @@ -637,7 +637,7 @@ _CONSTEXPR20 _NoThrowFwdIt destroy_n(_NoThrowFwdIt _First, const _Diff _Count_ra auto _UFirst = _STD _Get_unwrapped_n(_First, _Count); #if _HAS_CXX20 - if consteval { + if (_STD is_constant_evaluated()) { for (; _Count > 0; --_Count, (void) ++_UFirst) { _STD _Destroy_in_place(*_UFirst); } @@ -673,7 +673,7 @@ namespace ranges { } auto _UFirst = _STD _Get_unwrapped_n(_STD move(_First), _Count); - if consteval { + if (_STD is_constant_evaluated()) { do { _RANGES destroy_at(_STD addressof(*_UFirst)); ++_UFirst; diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 1e43553fb18..987e534e946 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1112,7 +1112,7 @@ _CONSTEXPR20 void _Destroy_range(_Alloc_ptr_t<_Alloc> _First, const _Alloc_ptr_t template _CONSTEXPR20 void _Destroy_range(_NoThrowFwdIt _First, const _NoThrowSentinel _Last) noexcept { #if _HAS_CXX20 - if consteval { + if (_STD is_constant_evaluated()) { for (; _First != _Last; ++_First) { _STD _Destroy_in_place(*_First); } diff --git a/tests/std/tests/P0784R7_library_support_for_more_constexpr_containers/test.cpp b/tests/std/tests/P0784R7_library_support_for_more_constexpr_containers/test.cpp index bc7ba1de7ce..8b5299caa1d 100644 --- a/tests/std/tests/P0784R7_library_support_for_more_constexpr_containers/test.cpp +++ b/tests/std/tests/P0784R7_library_support_for_more_constexpr_containers/test.cpp @@ -583,7 +583,7 @@ static_assert(!CanDestroyN); static_assert(!CanDestroyN); static_assert(!CanDestroyN); -#if defined(__clang__) && !defined(__EDG__) // TRANSITION, DevCom-10642767 (MSVC), DevCom-10896316 (EDG) +#ifdef __clang__ // TRANSITION, DevCom-10642767 (MSVC), DevCom-10896316 (EDG) // Test that destroy, destroy_at, and destroy_n properly destroy trivially destructible objects // during constant evaluation. // After such destruction, further access will cause core language undefined behavior, @@ -617,7 +617,7 @@ static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { ranges::dest static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { ranges::destroy_at(arr + 0); }>); static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { ranges::destroy_at(&arr); }>); static_assert(!CanWellDefinedlyAccessAfterOperation<[](auto& arr) { ranges::destroy_n(arr + 0, 1); }>); -#endif // defined(__clang__) && !defined(__EDG__) +#endif // ^^^ no workaround ^^^ int main() { test_runtime(1234);