https://gcc.gnu.org/g:1be3e4e43839d313364ffa99012ada41b4fae8da
commit r14-10705-g1be3e4e43839d313364ffa99012ada41b4fae8da Author: Jonathan Wakely <jwak...@redhat.com> Date: Wed Jul 10 23:14:19 2024 +0100 libstdc++: Fix std::allocator_traits::construct constraints [PR108619] Using std::is_constructible in the constraints introduces a spurious dependency on the type being destructible, which should not be required for constructing with an allocator. The test case shows a case where the type has a private destructor, which can be destroyed by the allocator, but std::is_destructible and std::is_constructible are false. Similarly, using is_nothrow_constructible in the noexcept-specifiers for the construct members of allocator_traits and std::allocator, __gnu_cxx::__new_allocator, and __gnu_cxx::__malloc_allocator gives the wrong answer if the type isn't destructible. We need a new type trait to define those correctly, so that we only check if the placement new-expression is nothrow after using is_constructible to check that it would be well-formed. On trunk all members of std::allocator_traits were rewritten in terms of 'if constexpr' using variable templates and the detection idiom. For the release branch this backport only changes the 'construct' member. Although we can use 'if constexpr' and variable templates in C++11 with appropriate uses of diagnostic pragmas, we can't have constexpr functions with multiple return statements. This means that in C++11 mode the _S_nothrow_construct helper used for noexcept-specifiers still needs to be a pair of overloads using enable_if. libstdc++-v3/ChangeLog: PR libstdc++/108619 * include/bits/alloc_traits.h (__allocator_traits_base): Add variable templates for detecting whether the allocator has a construct member, or if placement new can be used instead. (allocator_traits::__construct_helper): Remove. (allocator_traits::__has_construct): Remove. (allocator_traits::construct): Use 'if constexpr' instead of dispatching to overloads constrained with enable_if. (allocator_traits<allocator<T>>::construct): Use _Construct if construct_at is not supported. Use __is_nothrow_new_constructible for noexcept-specifier. (allocator_traits<allocator<void>>::construct): Use __is_nothrow_new_constructible for noexcept-specifier. * include/bits/new_allocator.h (construct): Likewise. * include/ext/malloc_allocator.h (construct): Likewise. * include/std/type_traits (__is_nothrow_new_constructible): New variable template. * testsuite/20_util/allocator/89510.cc: Adjust expected results. * testsuite/ext/malloc_allocator/89510.cc: Likewise. * testsuite/ext/new_allocator/89510.cc: Likewise. * testsuite/20_util/allocator_traits/members/108619.cc: New test. (cherry picked from commit 8cf51d7516b92b352c358c14ab4e456ae53c3371) Diff: --- libstdc++-v3/include/bits/alloc_traits.h | 131 +++++++++++++-------- libstdc++-v3/include/bits/new_allocator.h | 2 +- libstdc++-v3/include/ext/malloc_allocator.h | 2 +- libstdc++-v3/include/std/type_traits | 15 +++ libstdc++-v3/testsuite/20_util/allocator/89510.cc | 14 +-- .../20_util/allocator_traits/members/108619.cc | 35 ++++++ .../testsuite/ext/malloc_allocator/89510.cc | 14 +-- libstdc++-v3/testsuite/ext/new_allocator/89510.cc | 14 +-- 8 files changed, 154 insertions(+), 73 deletions(-) diff --git a/libstdc++-v3/include/bits/alloc_traits.h b/libstdc++-v3/include/bits/alloc_traits.h index 82fc79c7b9f9..a81b286eee70 100644 --- a/libstdc++-v3/include/bits/alloc_traits.h +++ b/libstdc++-v3/include/bits/alloc_traits.h @@ -48,6 +48,11 @@ namespace std _GLIBCXX_VISIBILITY(default) _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cplusplus >= 201103L + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wc++14-extensions" // for variable templates +#pragma GCC diagnostic ignored "-Wc++17-extensions" // for if-constexpr + /// @cond undocumented struct __allocator_traits_base { @@ -89,6 +94,39 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION using __pocs = typename _Tp::propagate_on_container_swap; template<typename _Tp> using __equal = __type_identity<typename _Tp::is_always_equal>; + + // __has_construct is true if a.construct(p, args...) is well-formed. + // __can_construct is true if either __has_construct is true, or if + // a placement new-expression for T(args...) is well-formed. We use this + // to constrain allocator_traits::construct, as a libstdc++ extension. + template<typename _Alloc, typename _Tp, typename... _Args> + using __construct_t + = decltype(std::declval<_Alloc&>().construct(std::declval<_Tp*>(), + std::declval<_Args>()...)); + template<typename _Alloc, typename _Tp, typename, typename... _Args> + static constexpr bool __has_construct_impl = false; + template<typename _Alloc, typename _Tp, typename... _Args> + static constexpr bool + __has_construct_impl<_Alloc, _Tp, + __void_t<__construct_t<_Alloc, _Tp, _Args...>>, + _Args...> + = true; + template<typename _Alloc, typename _Tp, typename... _Args> + static constexpr bool __has_construct + = __has_construct_impl<_Alloc, _Tp, void, _Args...>; + template<typename _Tp, typename... _Args> + using __new_expr_t + = decltype(::new((void*)0) _Tp(std::declval<_Args>()...)); + template<typename _Tp, typename, typename... _Args> + static constexpr bool __has_new_expr = false; + template<typename _Tp, typename... _Args> + static constexpr bool + __has_new_expr<_Tp, __void_t<__new_expr_t<_Tp, _Args...>>, _Args...> + = true; + template<typename _Alloc, typename _Tp, typename... _Args> + static constexpr bool __can_construct + = __has_construct<_Alloc, _Tp, _Args...> + || __has_new_expr<_Tp, void, _Args...>; }; template<typename _Alloc, typename _Up> @@ -242,43 +280,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _S_allocate(_Alloc2& __a, size_type __n, const_void_pointer, ...) { return __a.allocate(__n); } - template<typename _Tp, typename... _Args> - struct __construct_helper - { - template<typename _Alloc2, - typename = decltype(std::declval<_Alloc2*>()->construct( - std::declval<_Tp*>(), std::declval<_Args>()...))> - static true_type __test(int); - - template<typename> - static false_type __test(...); - - using type = decltype(__test<_Alloc>(0)); - }; - - template<typename _Tp, typename... _Args> - using __has_construct - = typename __construct_helper<_Tp, _Args...>::type; - - template<typename _Tp, typename... _Args> - static _GLIBCXX14_CONSTEXPR _Require<__has_construct<_Tp, _Args...>> - _S_construct(_Alloc& __a, _Tp* __p, _Args&&... __args) - noexcept(noexcept(__a.construct(__p, std::forward<_Args>(__args)...))) - { __a.construct(__p, std::forward<_Args>(__args)...); } - - template<typename _Tp, typename... _Args> - static _GLIBCXX14_CONSTEXPR - _Require<__and_<__not_<__has_construct<_Tp, _Args...>>, - is_constructible<_Tp, _Args...>>> - _S_construct(_Alloc&, _Tp* __p, _Args&&... __args) - noexcept(std::is_nothrow_constructible<_Tp, _Args...>::value) - { -#if __cplusplus <= 201703L - ::new((void*)__p) _Tp(std::forward<_Args>(__args)...); -#else - std::construct_at(__p, std::forward<_Args>(__args)...); -#endif - } template<typename _Alloc2, typename _Tp> static _GLIBCXX14_CONSTEXPR auto @@ -372,12 +373,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * arguments @a __args... */ template<typename _Tp, typename... _Args> - static _GLIBCXX20_CONSTEXPR auto + static _GLIBCXX20_CONSTEXPR + __enable_if_t<__can_construct<_Alloc, _Tp, _Args...>> construct(_Alloc& __a, _Tp* __p, _Args&&... __args) - noexcept(noexcept(_S_construct(__a, __p, - std::forward<_Args>(__args)...))) - -> decltype(_S_construct(__a, __p, std::forward<_Args>(__args)...)) - { _S_construct(__a, __p, std::forward<_Args>(__args)...); } + noexcept(_S_nothrow_construct<_Tp, _Args...>()) + { + if constexpr (__has_construct<_Alloc, _Tp, _Args...>) + __a.construct(__p, std::forward<_Args>(__args)...); + else + std::_Construct(__p, std::forward<_Args>(__args)...); + } /** * @brief Destroy an object of type @a _Tp @@ -416,7 +421,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static _GLIBCXX20_CONSTEXPR _Alloc select_on_container_copy_construction(const _Alloc& __rhs) { return _S_select(__rhs, 0); } + + private: +#if __cpp_constexpr >= 201304 // >= C++14 + template<typename _Tp, typename... _Args> + static constexpr bool + _S_nothrow_construct(_Alloc* __a = nullptr, _Tp* __p = nullptr) + { + if constexpr (__has_construct<_Alloc, _Tp, _Args...>) + return noexcept(__a->construct(__p, std::declval<_Args>()...)); + else + return __is_nothrow_new_constructible<_Tp, _Args...>; + } +#else + template<typename _Tp, typename... _Args> + static constexpr + __enable_if_t<__has_construct<_Alloc, _Tp, _Args...>, bool> + _S_nothrow_construct(_Alloc* __a = nullptr, _Tp* __p = nullptr) + { return noexcept(__a->construct(__p, std::declval<_Args>()...)); } + + template<typename _Tp, typename... _Args> + static constexpr + __enable_if_t<!__has_construct<_Alloc, _Tp, _Args...>, bool> + _S_nothrow_construct(_Alloc* = nullptr, _Tp* __p = nullptr) + { return __is_nothrow_new_constructible<_Tp, _Args...>; } +#endif }; +#pragma GCC diagnostic pop #if _GLIBCXX_HOSTED /// Partial specialization for std::allocator. @@ -526,14 +557,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Up, typename... _Args> [[__gnu__::__always_inline__]] static _GLIBCXX20_CONSTEXPR void - construct(allocator_type& __a __attribute__((__unused__)), _Up* __p, - _Args&&... __args) - noexcept(std::is_nothrow_constructible<_Up, _Args...>::value) + construct(allocator_type& __a __attribute__((__unused__)), + _Up* __p, _Args&&... __args) +#if __cplusplus <= 201703L + noexcept(noexcept(__a.construct(__p, std::forward<_Args>(__args)...))) +#else + noexcept(__is_nothrow_new_constructible<_Up, _Args...>) +#endif { #if __cplusplus <= 201703L __a.construct(__p, std::forward<_Args>(__args)...); -#else +#elif __cpp_constexpr_dynamic_alloc // >= C++20 std::construct_at(__p, std::forward<_Args>(__args)...); +#else + std::_Construct(__p, std::forward<_Args>(__args)...); #endif } @@ -653,7 +690,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION [[__gnu__::__always_inline__]] static _GLIBCXX20_CONSTEXPR void construct(allocator_type&, _Up* __p, _Args&&... __args) - noexcept(std::is_nothrow_constructible<_Up, _Args...>::value) + noexcept(__is_nothrow_new_constructible<_Up, _Args...>) { std::_Construct(__p, std::forward<_Args>(__args)...); } /** diff --git a/libstdc++-v3/include/bits/new_allocator.h b/libstdc++-v3/include/bits/new_allocator.h index 0e90c8819acd..21e7e22cae00 100644 --- a/libstdc++-v3/include/bits/new_allocator.h +++ b/libstdc++-v3/include/bits/new_allocator.h @@ -187,7 +187,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __attribute__((__always_inline__)) void construct(_Up* __p, _Args&&... __args) - noexcept(std::is_nothrow_constructible<_Up, _Args...>::value) + noexcept(__is_nothrow_new_constructible<_Up, _Args...>) { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } template<typename _Up> diff --git a/libstdc++-v3/include/ext/malloc_allocator.h b/libstdc++-v3/include/ext/malloc_allocator.h index b0a87b62d5ac..cde06d83241a 100644 --- a/libstdc++-v3/include/ext/malloc_allocator.h +++ b/libstdc++-v3/include/ext/malloc_allocator.h @@ -161,7 +161,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Up, typename... _Args> void construct(_Up* __p, _Args&&... __args) - noexcept(std::is_nothrow_constructible<_Up, _Args...>::value) + noexcept(std::__is_nothrow_new_constructible<_Up, _Args...>) { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } template<typename _Up> diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits index b441bf9908f7..fe5f4ee1bade 100644 --- a/libstdc++-v3/include/std/type_traits +++ b/libstdc++-v3/include/std/type_traits @@ -1597,6 +1597,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #endif #endif // __cpp_lib_is_nothrow_convertible +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wc++14-extensions" // for variable templates + template<typename _Tp, typename... _Args> + struct __is_nothrow_new_constructible_impl + : __bool_constant< + noexcept(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...)) + > + { }; + + template<typename _Tp, typename... _Args> + _GLIBCXX17_INLINE constexpr bool __is_nothrow_new_constructible + = __and_<is_constructible<_Tp, _Args...>, + __is_nothrow_new_constructible_impl<_Tp, _Args...>>::value; +#pragma GCC diagnostic pop + // Const-volatile modifications. /// remove_const diff --git a/libstdc++-v3/testsuite/20_util/allocator/89510.cc b/libstdc++-v3/testsuite/20_util/allocator/89510.cc index 95c85d2634ee..91526462a096 100644 --- a/libstdc++-v3/testsuite/20_util/allocator/89510.cc +++ b/libstdc++-v3/testsuite/20_util/allocator/89510.cc @@ -136,13 +136,11 @@ struct Z }; Z* zp; -// These construct calls should be noexcept, but they are false because -// they use is_nothrow_constructible which depends on is_nothrow_destructible. #if __cplusplus <= 201703L -static_assert( ! noexcept(a.construct(zp)), "wrong" ); -static_assert( ! noexcept(a.construct(zp, 1)), "wrong" ); -static_assert( ! noexcept(a.destroy(zp)), "" ); +static_assert( noexcept(a.construct(zp)), "" ); +static_assert( noexcept(a.construct(zp, 1)), "" ); +static_assert( ! noexcept(a.destroy(zp)), "~Z is noexcept(false)" ); #endif -static_assert( ! noexcept(AT::construct(a, zp)), "" ); -static_assert( ! noexcept(AT::construct(a, zp, 1)), "" ); -static_assert( ! noexcept(AT::destroy(a, zp)), "" ); +static_assert( noexcept(AT::construct(a, zp)), "" ); +static_assert( noexcept(AT::construct(a, zp, 1)), "" ); +static_assert( ! noexcept(AT::destroy(a, zp)), "~Z is noexcept(false)" ); diff --git a/libstdc++-v3/testsuite/20_util/allocator_traits/members/108619.cc b/libstdc++-v3/testsuite/20_util/allocator_traits/members/108619.cc new file mode 100644 index 000000000000..01bf611b8048 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/allocator_traits/members/108619.cc @@ -0,0 +1,35 @@ +// { dg-do compile { target c++11 } } + +#include <memory> + +template<typename T> +struct Alloc +{ + Alloc() = default; + + template<typename U> Alloc(const Alloc<U>&) { } + + using value_type = T; + + T* allocate(unsigned n) + { return std::allocator<T>().allocate(n); } + + void deallocate(T* p, unsigned n) + { return std::allocator<T>().deallocate(p, n); } + + template<typename U> void destroy(U* p){ p->~U(); } +}; + + +class S +{ + ~S() = default; + + friend Alloc<S>; +}; + +void +test_pr108619(Alloc<int> a, S* p) +{ + std::allocator_traits<Alloc<int>>::construct(a, p); +} diff --git a/libstdc++-v3/testsuite/ext/malloc_allocator/89510.cc b/libstdc++-v3/testsuite/ext/malloc_allocator/89510.cc index 1889c88d6e5a..771facbdf749 100644 --- a/libstdc++-v3/testsuite/ext/malloc_allocator/89510.cc +++ b/libstdc++-v3/testsuite/ext/malloc_allocator/89510.cc @@ -137,13 +137,11 @@ struct Z }; Z* zp; -// These construct calls should be noexcept, but they are false because -// they use is_nothrow_constructible which depends on is_nothrow_destructible. #if __cplusplus <= 201703L -static_assert( ! noexcept(a.construct(zp)), "wrong" ); -static_assert( ! noexcept(a.construct(zp, 1)), "wrong" ); -static_assert( ! noexcept(a.destroy(zp)), "" ); +static_assert( noexcept(a.construct(zp)), "" ); +static_assert( noexcept(a.construct(zp, 1)), "" ); +static_assert( ! noexcept(a.destroy(zp)), "~Z is noexcept(false)" ); #endif -static_assert( ! noexcept(AT::construct(a, zp)), "" ); -static_assert( ! noexcept(AT::construct(a, zp, 1)), "" ); -static_assert( ! noexcept(AT::destroy(a, zp)), "" ); +static_assert( noexcept(AT::construct(a, zp)), "" ); +static_assert( noexcept(AT::construct(a, zp, 1)), "" ); +static_assert( ! noexcept(AT::destroy(a, zp)), "~Z is noexcept(false)" ); diff --git a/libstdc++-v3/testsuite/ext/new_allocator/89510.cc b/libstdc++-v3/testsuite/ext/new_allocator/89510.cc index 06384ae55700..7fc443831c96 100644 --- a/libstdc++-v3/testsuite/ext/new_allocator/89510.cc +++ b/libstdc++-v3/testsuite/ext/new_allocator/89510.cc @@ -137,13 +137,11 @@ struct Z }; Z* zp; -// These construct calls should be noexcept, but they are false because -// they use is_nothrow_constructible which depends on is_nothrow_destructible. #if __cplusplus <= 201703L -static_assert( ! noexcept(a.construct(zp)), "wrong" ); -static_assert( ! noexcept(a.construct(zp, 1)), "wrong" ); -static_assert( ! noexcept(a.destroy(zp)), "" ); +static_assert( noexcept(a.construct(zp)), "" ); +static_assert( noexcept(a.construct(zp, 1)), "" ); +static_assert( ! noexcept(a.destroy(zp)), "~Z is noexcept(false)" ); #endif -static_assert( ! noexcept(AT::construct(a, zp)), "" ); -static_assert( ! noexcept(AT::construct(a, zp, 1)), "" ); -static_assert( ! noexcept(AT::destroy(a, zp)), "" ); +static_assert( noexcept(AT::construct(a, zp)), "" ); +static_assert( noexcept(AT::construct(a, zp, 1)), "" ); +static_assert( ! noexcept(AT::destroy(a, zp)), "~Z is noexcept(false)" );