From af959ca4aac256c8a9a20d2bfdf3bebec117064b Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 18 Dec 2023 00:26:27 +0800 Subject: [PATCH 1/5] Different control block types for `allocator_shared_for_overwrite` --- stl/inc/memory | 196 +++++++++++++++++++++++++----- tests/libcxx/expected_results.txt | 3 - 2 files changed, 167 insertions(+), 32 deletions(-) diff --git a/stl/inc/memory b/stl/inc/memory index d982b01e72f..76d11f95a89 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -1802,7 +1802,7 @@ private: template friend shared_ptr<_Ty0> _Make_shared_unbounded_array(size_t _Count, const _ArgTypes&... _Args); - template + template friend shared_ptr<_Ty0> _Allocate_shared_unbounded_array( const _Alloc& _Al, size_t _Count, const _ArgTypes&... _Args); #else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv @@ -2443,15 +2443,13 @@ public: explicit _Ref_count_obj_alloc3(const _Alloc& _Al_arg, _Types&&... _Args) : _Ebco_base<_Rebound>(_Al_arg), _Ref_count_base() { #if _HAS_CXX20 - if constexpr (sizeof...(_Types) == 1 && (is_same_v<_For_overwrite_tag, remove_cvref_t<_Types>> && ...)) { - _Default_construct_in_place(_Storage._Value); - ((void) _Args, ...); - } else -#endif // _HAS_CXX20 - { - allocator_traits<_Rebound>::construct( - this->_Get_val(), _STD addressof(_Storage._Value), _STD forward<_Types>(_Args)...); + if constexpr (sizeof...(_Types) == 1) { + // allocate_shared_for_overwrite should use another type for the control block + _STL_INTERNAL_STATIC_ASSERT(!(is_same_v<_For_overwrite_tag, remove_cvref_t<_Types>> && ...)); } +#endif // _HAS_CXX20 + allocator_traits<_Rebound>::construct( + this->_Get_val(), _STD addressof(_Storage._Value), _STD forward<_Types>(_Args)...); } union { @@ -2477,6 +2475,44 @@ private: }; #if _HAS_CXX20 +template +class _Ref_count_obj_alloc_for_overwrite : public _Ebco_base<_Rebind_alloc_t<_Alloc, _Ty>>, public _Ref_count_base { + // handle reference counting for object in control block, allocator + // initialize and destroy objects by the default mechanism +private: + static_assert(is_same_v<_Ty, remove_cv_t<_Ty>>, "allocate_shared_for_overwrite should remove_cv_t"); + + using _Rebound = _Rebind_alloc_t<_Alloc, _Ty>; + +public: + template + explicit _Ref_count_obj_alloc_for_overwrite(const _Alloc& _Al_arg) + : _Ebco_base<_Rebound>(_Al_arg), _Ref_count_base() { + _Default_construct_in_place(_Storage._Value); + } + + union { + _Wrap<_Ty> _Storage; + }; + +private: + ~_Ref_count_obj_alloc_for_overwrite() noexcept override { // TRANSITION, should be non-virtual + // nothing to do; _Storage._Value already destroyed by _Destroy() + + // See N4964 [class.dtor]/7. + } + + void _Destroy() noexcept override { // destroy managed resource + _Destroy_in_place(_Storage._Value); // use the default mechanism per LWG-4024 + } + + void _Delete_this() noexcept override { // destroy self + _Rebind_alloc_t<_Alloc, _Ref_count_obj_alloc_for_overwrite> _Al(this->_Get_val()); + this->~_Ref_count_obj_alloc_for_overwrite(); + _Deallocate_plain(_Al, this); + } +}; + template class _NODISCARD _Uninitialized_rev_destroying_backout_al { // class to undo partially constructed ranges in _Uninitialized_xxx_al algorithms @@ -2628,11 +2664,10 @@ public: template explicit _Ref_count_unbounded_array_alloc(const _Alloc& _Al_arg, const size_t _Count, const _Arg& _Val) : _Ebco_base<_Rebound>(_Al_arg), _Ref_count_base(), _Size(_Count) { - if constexpr (is_same_v<_For_overwrite_tag, _Arg>) { - _Uninitialized_default_construct_multidimensional_n(_Get_ptr(), _Size); // the allocator isn't needed - } else { - _Uninitialized_fill_multidimensional_n_al(_Get_ptr(), _Size, _Val, this->_Get_val()); - } + // allocate_shared_for_overwrite should use another type for the control block + _STL_INTERNAL_STATIC_ASSERT(!is_same_v<_For_overwrite_tag, _Arg>); + + _Uninitialized_fill_multidimensional_n_al(_Get_ptr(), _Size, _Val, this->_Get_val()); } _NODISCARD auto _Get_ptr() noexcept { @@ -2675,6 +2710,65 @@ private: } }; +template +class _Ref_count_unbounded_array_alloc_for_overwrite + : public _Ebco_base<_Rebind_alloc_t<_Alloc, remove_all_extents_t<_Ty>>>, + public _Ref_count_base { + // handle reference counting for unbounded array in control block, allocator + // initialize and destroy objects by the default mechanism +private: + static_assert(is_unbounded_array_v<_Ty>); + static_assert(is_same_v<_Ty, remove_cv_t<_Ty>>, "allocate_shared_for_overwrite should remove_cv_t"); + + using _Item = remove_all_extents_t<_Ty>; + using _Rebound = _Rebind_alloc_t<_Alloc, _Item>; + +public: + using _Element_type = remove_extent_t<_Ty>; + + explicit _Ref_count_unbounded_array_alloc_for_overwrite(const _Alloc& _Al_arg, const size_t _Count) + : _Ebco_base<_Rebound>(_Al_arg), _Ref_count_base(), _Size(_Count) { + _Uninitialized_default_construct_multidimensional_n(_Get_ptr(), _Size); // the allocator isn't needed + } + + _NODISCARD auto _Get_ptr() noexcept { + return _STD addressof(_Storage._Value); + } + +private: + size_t _Size; + + union { + _Wrap<_Element_type> _Storage; // flexible array must be last member + }; + + ~_Ref_count_unbounded_array_alloc_for_overwrite() noexcept override { // TRANSITION, should be non-virtual + // nothing to do; _Storage._Value already destroyed by _Destroy() + + // See N4964 [class.dtor]/7. + } + + void _Destroy() noexcept override { // destroy managed resource + _Reverse_destroy_multidimensional_n(_Get_ptr(), _Size); // use the default mechanism per LWG-4024 + } + + void _Delete_this() noexcept override { // destroy self + constexpr size_t _Align = alignof(_Ref_count_unbounded_array_alloc_for_overwrite); + using _Storage = _Alignas_storage_unit<_Align>; + using _Rebound_alloc = _Rebind_alloc_t<_Alloc, _Storage>; + + _Rebound_alloc _Al(this->_Get_val()); + const size_t _Bytes = _Calculate_bytes_for_flexible_array< // + _Ref_count_unbounded_array_alloc_for_overwrite, _Check_overflow::_Nope>(_Size); + const size_t _Storage_units = _Bytes / sizeof(_Storage); + + this->~_Ref_count_unbounded_array_alloc_for_overwrite(); + + _Al.deallocate(_STD _Refancy<_Alloc_ptr_t<_Rebound_alloc>>(reinterpret_cast<_Storage*>(this)), + static_cast<_Alloc_size_t<_Rebound_alloc>>(_Storage_units)); + } +}; + template class _Ref_count_bounded_array_alloc : public _Ebco_base<_Rebind_alloc_t<_Alloc, remove_all_extents_t<_Ty>>>, public _Ref_count_base { @@ -2695,12 +2789,10 @@ public: template explicit _Ref_count_bounded_array_alloc(const _Alloc& _Al_arg, const _Arg& _Val) : _Ebco_base<_Rebound>(_Al_arg), _Ref_count_base() { // don't value-initialize _Storage - if constexpr (is_same_v<_For_overwrite_tag, _Arg>) { - _Uninitialized_default_construct_multidimensional_n( - _Storage._Value, extent_v<_Ty>); // the allocator isn't needed - } else { - _Uninitialized_fill_multidimensional_n_al(_Storage._Value, extent_v<_Ty>, _Val, this->_Get_val()); - } + // allocate_shared_for_overwrite should use another type for the control block + _STL_INTERNAL_STATIC_ASSERT(!is_same_v<_For_overwrite_tag, _Arg>); + + _Uninitialized_fill_multidimensional_n_al(_Storage._Value, extent_v<_Ty>, _Val, this->_Get_val()); } union { @@ -2726,6 +2818,50 @@ private: _Deallocate_plain(_Al, this); } }; + +template +class _Ref_count_bounded_array_alloc_for_overwrite + : public _Ebco_base<_Rebind_alloc_t<_Alloc, remove_all_extents_t<_Ty>>>, + public _Ref_count_base { + // handle reference counting for bounded array in control block, allocator + // initialize and destroy objects by the default mechanism +private: + static_assert(is_bounded_array_v<_Ty>); + static_assert(is_same_v<_Ty, remove_cv_t<_Ty>>, "allocate_shared_overwrite should remove_cv_t"); + + using _Item = remove_all_extents_t<_Ty>; + using _Rebound = _Rebind_alloc_t<_Alloc, _Item>; + +public: + explicit _Ref_count_bounded_array_alloc_for_overwrite(const _Alloc& _Al_arg) + : _Ebco_base<_Rebound>(_Al_arg), _Ref_count_base() { // don't value-initialize _Storage + _Uninitialized_default_construct_multidimensional_n( + _Storage._Value, extent_v<_Ty>); // the allocator isn't needed + } + + union { + _Wrap<_Ty> _Storage; + }; + +private: + ~_Ref_count_bounded_array_alloc_for_overwrite() noexcept override { // TRANSITION, should be non-virtual + // nothing to do; _Storage._Value already destroyed by _Destroy() + + // See N4964 [class.dtor]/7. + } + + void _Destroy() noexcept override { // destroy managed resource + // not _Storage._Value as _Ty is an array type (not a class type or a scalar type), + // and thus cannot be used as a pseudo-destructor (N4964 [expr.prim.id.dtor]). + _Destroy_in_place(_Storage); // use the default mechanism per LWG-4024 + } + + void _Delete_this() noexcept override { // destroy self + _Rebind_alloc_t<_Alloc, _Ref_count_bounded_array_alloc_for_overwrite> _Al(this->_Get_val()); + this->~_Ref_count_bounded_array_alloc_for_overwrite(); + _Deallocate_plain(_Al, this); + } +}; #endif // _HAS_CXX20 _EXPORT_STD template @@ -2865,12 +3001,14 @@ struct _Allocate_n_ptr { _Allocate_n_ptr& operator=(const _Allocate_n_ptr&) = delete; }; -_EXPORT_STD /* TRANSITION, VSO-1538698 */ template +_EXPORT_STD /* TRANSITION, VSO-1538698 */ template _NODISCARD shared_ptr<_Ty> _Allocate_shared_unbounded_array( const _Alloc& _Al, const size_t _Count, const _ArgTypes&... _Args) { // make a shared_ptr to an unbounded array static_assert(is_unbounded_array_v<_Ty>); - using _Refc = _Ref_count_unbounded_array_alloc, _Alloc>; + using _Refc = conditional_t<_IsForOverwrite, // + _Ref_count_unbounded_array_alloc_for_overwrite, _Alloc>, + _Ref_count_unbounded_array_alloc, _Alloc>>; constexpr size_t _Align = alignof(_Refc); using _Storage = _Alignas_storage_unit<_Align>; _Rebind_alloc_t<_Alloc, _Storage> _Rebound(_Al); @@ -2888,13 +3026,13 @@ _NODISCARD shared_ptr<_Ty> _Allocate_shared_unbounded_array( _EXPORT_STD template _NODISCARD_SMART_PTR_ALLOC enable_if_t, shared_ptr<_Ty>> allocate_shared( const _Alloc& _Al, const size_t _Count) { - return _Allocate_shared_unbounded_array<_Ty>(_Al, _Count); + return _Allocate_shared_unbounded_array(_Al, _Count); } _EXPORT_STD template _NODISCARD_SMART_PTR_ALLOC enable_if_t, shared_ptr<_Ty>> allocate_shared( const _Alloc& _Al, const size_t _Count, const remove_extent_t<_Ty>& _Val) { - return _Allocate_shared_unbounded_array<_Ty>(_Al, _Count, _Val); + return _Allocate_shared_unbounded_array(_Al, _Count, _Val); } _EXPORT_STD template @@ -2934,22 +3072,22 @@ _NODISCARD_SMART_PTR_ALLOC enable_if_t, shared_ptr<_T shared_ptr<_Ty> _Ret; if constexpr (is_array_v<_Ty>) { // make a shared_ptr to a bounded array - using _Refc = _Ref_count_bounded_array_alloc, _Alloc>; + using _Refc = _Ref_count_bounded_array_alloc_for_overwrite, _Alloc>; using _Alblock = _Rebind_alloc_t<_Alloc, _Refc>; _Alblock _Rebound(_Al); _Alloc_construct_ptr _Constructor{_Rebound}; _Constructor._Allocate(); - ::new (_STD _Voidify_unfancy(_Constructor._Ptr)) _Refc(_Al, _For_overwrite_tag{}); + ::new (_STD _Voidify_unfancy(_Constructor._Ptr)) _Refc(_Al); const auto _Ptr = static_cast*>(_Constructor._Ptr->_Storage._Value); _Ret._Set_ptr_rep_and_enable_shared(_Ptr, _STD _Unfancy(_Constructor._Release())); } else { // make a shared_ptr to non-array object - using _Refoa = _Ref_count_obj_alloc3, _Alloc>; + using _Refoa = _Ref_count_obj_alloc_for_overwrite, _Alloc>; using _Alblock = _Rebind_alloc_t<_Alloc, _Refoa>; _Alblock _Rebound(_Al); _Alloc_construct_ptr<_Alblock> _Constructor{_Rebound}; _Constructor._Allocate(); - _STD _Construct_in_place(*_Constructor._Ptr, _Al, _For_overwrite_tag{}); + _STD _Construct_in_place(*_Constructor._Ptr, _Al); const auto _Ptr = reinterpret_cast<_Ty*>(_STD addressof(_Constructor._Ptr->_Storage._Value)); _Ret._Set_ptr_rep_and_enable_shared(_Ptr, _STD _Unfancy(_Constructor._Release())); } @@ -2960,7 +3098,7 @@ _NODISCARD_SMART_PTR_ALLOC enable_if_t, shared_ptr<_T _EXPORT_STD template _NODISCARD_SMART_PTR_ALLOC enable_if_t, shared_ptr<_Ty>> allocate_shared_for_overwrite( const _Alloc& _Al, const size_t _Count) { - return _Allocate_shared_unbounded_array<_Ty>(_Al, _Count, _For_overwrite_tag{}); + return _Allocate_shared_unbounded_array(_Al, _Count); } #endif // _HAS_CXX20 diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index aa7119f94a3..861c6feefbc 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -1167,9 +1167,6 @@ std/localization/locales/locale/locale.cons/name_construction.pass.cpp FAIL # Not analyzed. After LLVM-74630 fixes LLVM-74214, will be blocked by our ctype_base deriving from locale::facet. std/localization/locale.categories/category.numeric/locale.num.get/user_defined_char_type.pass.cpp FAIL -# Not analyzed. Assertion failed: alloc_stats.destroy_count == 0 -std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared_for_overwrite.pass.cpp FAIL - # Not analyzed. Assertion failed: res == cvt.ok std/localization/codecvt_unicode.pass.cpp FAIL From df8da39d6ea353304ce576377d91dae578fb9b6d Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 18 Dec 2023 11:55:41 +0800 Subject: [PATCH 2/5] Visualizer for `_Ref_count_obj_alloc_for_overwrite` --- stl/debugger/STL.natvis | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/stl/debugger/STL.natvis b/stl/debugger/STL.natvis index a3047ef51d3..86dc4e0231e 100644 --- a/stl/debugger/STL.natvis +++ b/stl/debugger/STL.natvis @@ -386,6 +386,19 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + + + + + + + allocate_shared_for_overwrite + + ($T1 *) &_Storage + allocator() + + + custom deleter From 138b09fe5434fbd763559648806ee7e9e346146a Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sun, 24 Dec 2023 22:14:54 +0800 Subject: [PATCH 3/5] Complete visualizers for control blocks for arrays --- stl/debugger/STL.natvis | 93 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 92 insertions(+), 1 deletion(-) diff --git a/stl/debugger/STL.natvis b/stl/debugger/STL.natvis index 86dc4e0231e..01e8dd606b6 100644 --- a/stl/debugger/STL.natvis +++ b/stl/debugger/STL.natvis @@ -362,6 +362,35 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + + + make_shared<T[N]> + + _Storage._Value + + + + + + + + make_shared<T[]> + + + size() + data() + + + + + + + make_shared<T[]> + + _Storage._Value + + + allocate_shared @@ -386,7 +415,38 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - + + + + + + + allocate_shared<T[N]> + + _Storage._Value + allocator() + + + + + + + + + + + + allocate_shared<T[]> + + allocator() + + size() + data() + + + + + @@ -399,6 +459,37 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + + + + + + + allocate_shared_for_overwrite<T[N]> + + _Storage._Value + allocator() + + + + + + + + + + + + allocate_shared_for_overwrite<T[]> + + allocator() + + size() + data() + + + + custom deleter From dd8f84874caac1b9e6de30502c71d0b1a0c81b8a Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 25 Jan 2024 06:19:36 -0800 Subject: [PATCH 4/5] Guard with `defined(_ENABLE_STL_INTERNAL_CHECK)`. --- stl/inc/memory | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/memory b/stl/inc/memory index 721c4ba0a89..fa308a65f9d 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -2461,12 +2461,12 @@ public: template explicit _Ref_count_obj_alloc3(const _Alloc& _Al_arg, _Types&&... _Args) : _Ebco_base<_Rebound>(_Al_arg), _Ref_count_base() { -#if _HAS_CXX20 +#if _HAS_CXX20 && defined(_ENABLE_STL_INTERNAL_CHECK) if constexpr (sizeof...(_Types) == 1) { // allocate_shared_for_overwrite should use another type for the control block _STL_INTERNAL_STATIC_ASSERT(!(is_same_v<_For_overwrite_tag, remove_cvref_t<_Types>> && ...)); } -#endif // _HAS_CXX20 +#endif // _HAS_CXX20 && defined(_ENABLE_STL_INTERNAL_CHECK) allocator_traits<_Rebound>::construct( this->_Get_val(), _STD addressof(_Storage._Value), _STD forward<_Types>(_Args)...); } From 057ec406595f323e50af6cd31c0f00902a63adaa Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 25 Jan 2024 06:20:08 -0800 Subject: [PATCH 5/5] Fix typo. --- stl/inc/memory | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/memory b/stl/inc/memory index fa308a65f9d..8ad20eb7cde 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -2846,7 +2846,7 @@ class _Ref_count_bounded_array_alloc_for_overwrite // initialize and destroy objects by the default mechanism private: static_assert(is_bounded_array_v<_Ty>); - static_assert(is_same_v<_Ty, remove_cv_t<_Ty>>, "allocate_shared_overwrite should remove_cv_t"); + static_assert(is_same_v<_Ty, remove_cv_t<_Ty>>, "allocate_shared_for_overwrite should remove_cv_t"); using _Item = remove_all_extents_t<_Ty>; using _Rebound = _Rebind_alloc_t<_Alloc, _Item>;