[libc++] LWG3187: P0591R4 reverted DR 2586 fixes to scoped_allocator_adaptor::construct()
#152424
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: A. Jiang (frederick-vs-ja) ChangesApplying it (adding missing Also
Fixes #100256. Full diff: https://github.com/llvm/llvm-project/pull/152424.diff 6 Files Affected:
diff --git a/libcxx/docs/Status/Cxx20Issues.csv b/libcxx/docs/Status/Cxx20Issues.csv
index 98b49f92bdf7a..7b5bcd69f6abd 100644
--- a/libcxx/docs/Status/Cxx20Issues.csv
+++ b/libcxx/docs/Status/Cxx20Issues.csv
@@ -151,7 +151,7 @@
"`LWG3184 <https://wg21.link/LWG3184>`__","Inconsistencies in ``bind_front``\ wording","2019-07 (Cologne)","|Complete|","13",""
"`LWG3185 <https://wg21.link/LWG3185>`__","Uses-allocator construction functions missing ``constexpr``\ and ``noexcept``\ ","2019-07 (Cologne)","|Complete|","16",""
"`LWG3186 <https://wg21.link/LWG3186>`__","``ranges``\ removal, partition, and ``partial_sort_copy``\ algorithms discard useful information","2019-07 (Cologne)","|Complete|","15",""
-"`LWG3187 <https://wg21.link/LWG3187>`__","`P0591R4 <https://wg21.link/p0591r4>`__ reverted DR 2586 fixes to ``scoped_allocator_adaptor::construct()``\ ","2019-07 (Cologne)","","",""
+"`LWG3187 <https://wg21.link/LWG3187>`__","`P0591R4 <https://wg21.link/p0591r4>`__ reverted DR 2586 fixes to ``scoped_allocator_adaptor::construct()``\ ","2019-07 (Cologne)","|Complete|","22",""
"`LWG3191 <https://wg21.link/LWG3191>`__","``std::ranges::shuffle``\ synopsis does not match algorithm definition","2019-07 (Cologne)","|Complete|","15",""
"`LWG3196 <https://wg21.link/LWG3196>`__","``std::optional<T>``\ is ill-formed is ``T``\ is an array","2019-07 (Cologne)","|Complete|","",""
"`LWG3198 <https://wg21.link/LWG3198>`__","Bad constraint on ``std::span::span()``\ ","2019-07 (Cologne)","|Complete|","",""
diff --git a/libcxx/include/__memory/allocator_arg_t.h b/libcxx/include/__memory/allocator_arg_t.h
index 31a73fc4557ef..a3dac879f45b6 100644
--- a/libcxx/include/__memory/allocator_arg_t.h
+++ b/libcxx/include/__memory/allocator_arg_t.h
@@ -14,6 +14,7 @@
#include <__memory/uses_allocator.h>
#include <__type_traits/integral_constant.h>
#include <__type_traits/is_constructible.h>
+#include <__type_traits/remove_cv.h>
#include <__type_traits/remove_cvref.h>
#include <__utility/forward.h>
@@ -40,34 +41,15 @@ constexpr allocator_arg_t allocator_arg = allocator_arg_t();
template <class _Tp, class _Alloc, class... _Args>
struct __uses_alloc_ctor_imp {
using _RawAlloc _LIBCPP_NODEBUG = __remove_cvref_t<_Alloc>;
- static const bool __ua = uses_allocator<_Tp, _RawAlloc>::value;
- static const bool __ic = is_constructible<_Tp, allocator_arg_t, _Alloc, _Args...>::value;
- static const int value = __ua ? 2 - __ic : 0;
+ static constexpr bool __ua = uses_allocator<__remove_cv_t<_Tp>, _RawAlloc>::value;
+ static constexpr bool __ic_head = is_constructible<_Tp, allocator_arg_t, const _RawAlloc&, _Args...>::value;
+ static constexpr bool __ic_tail = is_constructible<_Tp, _Args..., const _RawAlloc&>::value;
+ static constexpr int value = __ua ? (__ic_head ? 1 : __ic_tail ? 2 : -1) : 0;
};
template <class _Tp, class _Alloc, class... _Args>
struct __uses_alloc_ctor : integral_constant<int, __uses_alloc_ctor_imp<_Tp, _Alloc, _Args...>::value> {};
-template <class _Tp, class _Allocator, class... _Args>
-inline _LIBCPP_HIDE_FROM_ABI void
-__user_alloc_construct_impl(integral_constant<int, 0>, _Tp* __storage, const _Allocator&, _Args&&... __args) {
- new (__storage) _Tp(std::forward<_Args>(__args)...);
-}
-
-// FIXME: This should have a version which takes a non-const alloc.
-template <class _Tp, class _Allocator, class... _Args>
-inline _LIBCPP_HIDE_FROM_ABI void
-__user_alloc_construct_impl(integral_constant<int, 1>, _Tp* __storage, const _Allocator& __a, _Args&&... __args) {
- new (__storage) _Tp(allocator_arg, __a, std::forward<_Args>(__args)...);
-}
-
-// FIXME: This should have a version which takes a non-const alloc.
-template <class _Tp, class _Allocator, class... _Args>
-inline _LIBCPP_HIDE_FROM_ABI void
-__user_alloc_construct_impl(integral_constant<int, 2>, _Tp* __storage, const _Allocator& __a, _Args&&... __args) {
- new (__storage) _Tp(std::forward<_Args>(__args)..., __a);
-}
-
#endif // _LIBCPP_CXX03_LANG
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/__memory/uses_allocator_construction.h b/libcxx/include/__memory/uses_allocator_construction.h
index 6733f5cf6fc35..55b634f5e7d0e 100644
--- a/libcxx/include/__memory/uses_allocator_construction.h
+++ b/libcxx/include/__memory/uses_allocator_construction.h
@@ -16,6 +16,7 @@
#include <__type_traits/enable_if.h>
#include <__type_traits/remove_cv.h>
#include <__utility/declval.h>
+#include <__utility/integer_sequence.h>
#include <__utility/pair.h>
#include <__utility/piecewise_construct.h>
#include <tuple>
@@ -29,6 +30,43 @@ _LIBCPP_PUSH_MACROS
_LIBCPP_BEGIN_NAMESPACE_STD
+// TODO: Guard this furtherly with _LIBCPP_STD_VER < 20 once P0591R4 is fully implemented.
+#if !defined(_LIBCPP_CXX03_LANG)
+
+template <class _Alloc, class... _Args, size_t... _Is>
+_LIBCPP_HIDE_FROM_ABI void __transform_tuple_using_allocator_impl(
+ integral_constant<int, -1>, const _Alloc&, tuple<_Args...>&&, __index_sequence<_Is...>) {
+ static_assert(false, "If uses_allocator_v<T, A> is true, T has to be allocator-constructible");
+}
+
+template <class _Alloc, class... _Args, size_t... _Is>
+_LIBCPP_HIDE_FROM_ABI tuple<_Args&&...> __transform_tuple_using_allocator_impl(
+ integral_constant<int, 0>, const _Alloc&, tuple<_Args...>&& __t, __index_sequence<_Is...>) {
+ return tuple<_Args&&...>(std::move(__t));
+}
+
+template <class _Alloc, class... _Args, size_t... _Is>
+_LIBCPP_HIDE_FROM_ABI tuple<allocator_arg_t, const _Alloc&, _Args&&...> __transform_tuple_using_allocator_impl(
+ integral_constant<int, 1>, const _Alloc& __a, tuple<_Args...>&& __t, __index_sequence<_Is...>) {
+ return tuple<allocator_arg_t, const _Alloc&, _Args&&...>(allocator_arg, __a, std::get<_Is>(std::move(__t))...);
+}
+
+template <class _Alloc, class... _Args, size_t... _Is>
+_LIBCPP_HIDE_FROM_ABI tuple<_Args&&..., const _Alloc&> __transform_tuple_using_allocator_impl(
+ integral_constant<int, 2>, const _Alloc& __a, tuple<_Args...>&& __t, __index_sequence<_Is...>) {
+ return tuple<_Args&&..., const _Alloc&>(std::get<_Is>(std::move(__t))..., __a);
+}
+
+template <class _Tp, class _Alloc, class... _Args>
+_LIBCPP_HIDE_FROM_ABI auto __transform_tuple_using_allocator(const _Alloc& __a, tuple<_Args...>&& __t)
+ -> decltype(std::__transform_tuple_using_allocator_impl(
+ __uses_alloc_ctor<_Tp, _Alloc, _Args>{}, __a, std::move(__t), __make_index_sequence<sizeof...(_Args)>{})) {
+ return std::__transform_tuple_using_allocator_impl(
+ __uses_alloc_ctor<_Tp, _Alloc, _Args>{}, __a, std::move(__t), __make_index_sequence<sizeof...(_Args)>{});
+}
+
+#endif // !defined(_LIBCPP_CXX03_LANG)
+
#if _LIBCPP_STD_VER >= 17
template <class _Tp>
diff --git a/libcxx/include/__memory_resource/polymorphic_allocator.h b/libcxx/include/__memory_resource/polymorphic_allocator.h
index 9a351199b5b16..ec38a3469675b 100644
--- a/libcxx/include/__memory_resource/polymorphic_allocator.h
+++ b/libcxx/include/__memory_resource/polymorphic_allocator.h
@@ -14,9 +14,15 @@
#include <__cstddef/byte.h>
#include <__cstddef/max_align_t.h>
#include <__fwd/pair.h>
+#include <__memory/allocator_arg_t.h>
+#include <__memory/uses_allocator.h>
+#include <__memory/uses_allocator_construction.h>
#include <__memory_resource/memory_resource.h>
#include <__new/exceptions.h>
#include <__new/placement_new_delete.h>
+#include <__type_traits/is_constructible.h>
+#include <__type_traits/remove_cv.h>
+#include <__utility/as_const.h>
#include <__utility/exception_guard.h>
#include <__utility/piecewise_construct.h>
#include <limits>
@@ -122,11 +128,18 @@ class _LIBCPP_AVAILABILITY_PMR polymorphic_allocator {
template <class _Tp, class... _Ts>
_LIBCPP_HIDE_FROM_ABI void construct(_Tp* __p, _Ts&&... __args) {
- std::__user_alloc_construct_impl(
- typename __uses_alloc_ctor<_Tp, polymorphic_allocator&, _Ts...>::type(),
- __p,
- *this,
- std::forward<_Ts>(__args)...);
+ if constexpr (!uses_allocator_v<remove_cv_t<_Tp>, polymorphic_allocator>) {
+ static_assert(is_constructible_v<_Tp, _Ts...>,
+ "If uses_allocator_v<T, polymorphic_allocator> is false, T has to be constructible from arguments");
+ ::new ((void*)__p) _Tp(std::forward<_Ts>(__args)...);
+ } else if constexpr (is_constructible_v<_Tp, allocator_arg_t, const polymorphic_allocator&, _Ts...>) {
+ ::new ((void*)__p) _Tp(allocator_arg, std::as_const(*this), std::forward<_Ts>(__args)...);
+ } else if constexpr (is_constructible_v<_Tp, _Ts..., const polymorphic_allocator&>) {
+ ::new ((void*)__p) _Tp(std::forward<_Ts>(__args)..., std::as_const(*this));
+ } else {
+ static_assert(
+ false, "If uses_allocator_v<T, polymorphic_allocator> is true, T has to be allocator-constructible");
+ }
}
template <class _T1, class _T2, class... _Args1, class... _Args2>
@@ -134,12 +147,8 @@ class _LIBCPP_AVAILABILITY_PMR polymorphic_allocator {
construct(pair<_T1, _T2>* __p, piecewise_construct_t, tuple<_Args1...> __x, tuple<_Args2...> __y) {
::new ((void*)__p) pair<_T1, _T2>(
piecewise_construct,
- __transform_tuple(typename __uses_alloc_ctor< _T1, polymorphic_allocator&, _Args1... >::type(),
- std::move(__x),
- make_index_sequence<sizeof...(_Args1)>()),
- __transform_tuple(typename __uses_alloc_ctor< _T2, polymorphic_allocator&, _Args2... >::type(),
- std::move(__y),
- make_index_sequence<sizeof...(_Args2)>()));
+ std::__transform_tuple_using_allocator<_T1>(*this, std::move(__x)),
+ std::__transform_tuple_using_allocator<_T2>(*this, std::move(__y)));
}
template <class _T1, class _T2>
@@ -193,26 +202,6 @@ class _LIBCPP_AVAILABILITY_PMR polymorphic_allocator {
# endif
private:
- template <class... _Args, size_t... _Is>
- _LIBCPP_HIDE_FROM_ABI tuple<_Args&&...>
- __transform_tuple(integral_constant<int, 0>, tuple<_Args...>&& __t, index_sequence<_Is...>) {
- return std::forward_as_tuple(std::get<_Is>(std::move(__t))...);
- }
-
- template <class... _Args, size_t... _Is>
- _LIBCPP_HIDE_FROM_ABI tuple<allocator_arg_t const&, polymorphic_allocator&, _Args&&...>
- __transform_tuple(integral_constant<int, 1>, tuple<_Args...>&& __t, index_sequence<_Is...>) {
- using _Tup = tuple<allocator_arg_t const&, polymorphic_allocator&, _Args&&...>;
- return _Tup(allocator_arg, *this, std::get<_Is>(std::move(__t))...);
- }
-
- template <class... _Args, size_t... _Is>
- _LIBCPP_HIDE_FROM_ABI tuple<_Args&&..., polymorphic_allocator&>
- __transform_tuple(integral_constant<int, 2>, tuple<_Args...>&& __t, index_sequence<_Is...>) {
- using _Tup = tuple<_Args&&..., polymorphic_allocator&>;
- return _Tup(std::get<_Is>(std::move(__t))..., *this);
- }
-
_LIBCPP_HIDE_FROM_ABI size_t __max_size() const noexcept {
return numeric_limits<size_t>::max() / sizeof(value_type);
}
diff --git a/libcxx/include/scoped_allocator b/libcxx/include/scoped_allocator
index 74effc547f3e2..d152a7ade498b 100644
--- a/libcxx/include/scoped_allocator
+++ b/libcxx/include/scoped_allocator
@@ -113,12 +113,15 @@ template <class OuterA1, class OuterA2, class... InnerAllocs>
# include <__cxx03/__config>
#else
# include <__config>
+# include <__memory/allocator_arg_t.h>
# include <__memory/allocator_traits.h>
+# include <__memory/uses_allocator.h>
# include <__memory/uses_allocator_construction.h>
# include <__type_traits/common_type.h>
# include <__type_traits/enable_if.h>
# include <__type_traits/integral_constant.h>
# include <__type_traits/is_constructible.h>
+# include <__type_traits/remove_cv.h>
# include <__type_traits/remove_reference.h>
# include <__utility/declval.h>
# include <__utility/forward.h>
@@ -421,23 +424,30 @@ public:
# else
template <class _Tp, class... _Args>
_LIBCPP_HIDE_FROM_ABI void construct(_Tp* __p, _Args&&... __args) {
- __construct(__uses_alloc_ctor<_Tp, inner_allocator_type&, _Args...>(), __p, std::forward<_Args>(__args)...);
+ using _OM = __outermost<outer_allocator_type>;
+ if constexpr (!uses_allocator<__remove_cv_t<_Type>, inner_allocator_type>::value) {
+ allocator_traits<typename _OM::type>::construct(_OM()(outer_allocator()), __p, std::forward<_Args>(__args)...);
+ } else if constexpr (is_constructible<_Type, allocator_arg_t, const inner_allocator_type&, _Args...>::value) {
+ allocator_traits<typename _OM::type>::construct(
+ _OM()(outer_allocator()), __p, allocator_arg, inner_allocator(), std::forward<_Args>(__args)...);
+ } else if constexpr (is_constructible<_Type, _Args..., const inner_allocator_type&>::value) {
+ allocator_traits<typename _OM::type>::construct(
+ _OM()(outer_allocator()), __p, std::forward<_Args>(__args)..., inner_allocator());
+ } else {
+ static_assert(false, "If uses_allocator_v<T, inner_allocator_type> is true, T has to be allocator-constructible");
+ }
}
template <class _T1, class _T2, class... _Args1, class... _Args2>
_LIBCPP_HIDE_FROM_ABI void
construct(pair<_T1, _T2>* __p, piecewise_construct_t, tuple<_Args1...> __x, tuple<_Args2...> __y) {
- typedef __outermost<outer_allocator_type> _OM;
+ using _OM = __outermost<outer_allocator_type>;
allocator_traits<typename _OM::type>::construct(
_OM()(outer_allocator()),
__p,
piecewise_construct,
- __transform_tuple(typename __uses_alloc_ctor< _T1, inner_allocator_type&, _Args1... >::type(),
- std::move(__x),
- __make_index_sequence<sizeof...(_Args1)>()),
- __transform_tuple(typename __uses_alloc_ctor< _T2, inner_allocator_type&, _Args2... >::type(),
- std::move(__y),
- __make_index_sequence<sizeof...(_Args2)>()));
+ std::__transform_tuple_using_allocator<_T1>(inner_allocator(), std::move(__x)),
+ std::__transform_tuple_using_allocator<_T2>(inner_allocator(), std::move(__y)));
}
template <class _T1, class _T2>
@@ -481,46 +491,6 @@ private:
_LIBCPP_HIDE_FROM_ABI explicit scoped_allocator_adaptor(
outer_allocator_type&& __o, inner_allocator_type&& __i) _NOEXCEPT : _Base(std::move(__o), std::move(__i)) {}
- template <class _Tp, class... _Args>
- _LIBCPP_HIDE_FROM_ABI void __construct(integral_constant<int, 0>, _Tp* __p, _Args&&... __args) {
- typedef __outermost<outer_allocator_type> _OM;
- allocator_traits<typename _OM::type>::construct(_OM()(outer_allocator()), __p, std::forward<_Args>(__args)...);
- }
-
- template <class _Tp, class... _Args>
- _LIBCPP_HIDE_FROM_ABI void __construct(integral_constant<int, 1>, _Tp* __p, _Args&&... __args) {
- typedef __outermost<outer_allocator_type> _OM;
- allocator_traits<typename _OM::type>::construct(
- _OM()(outer_allocator()), __p, allocator_arg, inner_allocator(), std::forward<_Args>(__args)...);
- }
-
- template <class _Tp, class... _Args>
- _LIBCPP_HIDE_FROM_ABI void __construct(integral_constant<int, 2>, _Tp* __p, _Args&&... __args) {
- typedef __outermost<outer_allocator_type> _OM;
- allocator_traits<typename _OM::type>::construct(
- _OM()(outer_allocator()), __p, std::forward<_Args>(__args)..., inner_allocator());
- }
-
- template <class... _Args, size_t... _Idx>
- _LIBCPP_HIDE_FROM_ABI tuple<_Args&&...>
- __transform_tuple(integral_constant<int, 0>, tuple<_Args...>&& __t, __index_sequence<_Idx...>) {
- return std::forward_as_tuple(std::get<_Idx>(std::move(__t))...);
- }
-
- template <class... _Args, size_t... _Idx>
- _LIBCPP_HIDE_FROM_ABI tuple<allocator_arg_t, inner_allocator_type&, _Args&&...>
- __transform_tuple(integral_constant<int, 1>, tuple<_Args...>&& __t, __index_sequence<_Idx...>) {
- using _Tup = tuple<allocator_arg_t, inner_allocator_type&, _Args&&...>;
- return _Tup(allocator_arg, inner_allocator(), std::get<_Idx>(std::move(__t))...);
- }
-
- template <class... _Args, size_t... _Idx>
- _LIBCPP_HIDE_FROM_ABI tuple<_Args&&..., inner_allocator_type&>
- __transform_tuple(integral_constant<int, 2>, tuple<_Args...>&& __t, __index_sequence<_Idx...>) {
- using _Tup = tuple<_Args&&..., inner_allocator_type&>;
- return _Tup(std::get<_Idx>(std::move(__t))..., inner_allocator());
- }
-
template <class...>
friend class __scoped_allocator_storage;
};
diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index be30ab5b2173d..27c13fa63e0a9 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -382,6 +382,11 @@ public:
static_assert(!is_reference<_Hp>::value, "Attempted to default construct a reference element in a tuple");
}
+ template <class _Alloc, class... _Args>
+ _LIBCPP_HIDE_FROM_ABI __tuple_leaf(integral_constant<int, -1>, const _Alloc&, _Args&&...) {
+ static_assert(false, "If uses_allocator_v<T, A> is true, T has to be allocator-constructible");
+ }
+
template <class _Alloc>
_LIBCPP_HIDE_FROM_ABI constexpr __tuple_leaf(integral_constant<int, 0>, const _Alloc&) : __value_() {
static_assert(!is_reference<_Hp>::value, "Attempted to default construct a reference element in a tuple");
@@ -456,6 +461,11 @@ public:
_LIBCPP_HIDE_FROM_ABI constexpr __tuple_leaf() noexcept(is_nothrow_default_constructible<_Hp>::value) {}
+ template <class _Alloc, class... _Args>
+ _LIBCPP_HIDE_FROM_ABI __tuple_leaf(integral_constant<int, -1>, const _Alloc&, _Args&&...) {
+ static_assert(false, "If uses_allocator_v<T, A> is true, T has to be allocator-constructible");
+ }
+
template <class _Alloc>
_LIBCPP_HIDE_FROM_ABI constexpr __tuple_leaf(integral_constant<int, 0>, const _Alloc&) {}
|
frederick-vs-ja
force-pushed
the
lwg-3187
branch
3 times, most recently
from
70249af to
79c76f3
Compare
frederick-vs-ja
commented
Aug 7, 2025
frederick-vs-ja
force-pushed
the
lwg-3187
branch
2 times, most recently
from
cfef8fb to
fce20de
Compare
... `scoped_allocator_adaptor::construct()` Applying it (adding missing `const``) to pre-C++20 dispatching mechanisms. Also - make dispatch mechanisms in pre-C++20 `construct` more consistent, and - add fallback dispatching and overloads to emit better error messages.
frederick-vs-ja
force-pushed
the
lwg-3187
branch
from
fce20de to
af9740d
Compare
philnik777
requested changes
Aug 7, 2025
I found that there're some behavior differences introduced by P0591R4 and I'm not sure whether the C++20 behavior should be backported.
|
I think we should rather modify the utilities to have the required behaviour in C++17. Using them in these places was the original intent after all. Right now we don't fully implement P0591R4, since we never updated |
Oh, for |
Urgh. WDYT about implementing the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Applying it (adding missing
const) to pre-C++20 dispatching mechanisms.Also
constructmore consistent, andFixes #100256.