On Thu, 18 Sep 2025 at 18:52 -0700, Ben Wu wrote:
Bootstrapped and tested with check-target-libstdc++-v3 on
x86_64-pc-linux-gnu.
Could someone help review and commit?
For applying, please use the attached patch file since my email client has
not preserved
the tabs.
Thanks,
-Ben
-- 8< --
In order to emplace a value in the middle of a deque, a temporary was
previously constructed directly with __args... in _M_emplace_aux.
This would not work since std::deque is allocator-aware and should
construct elements with _Alloc_traits::construct instead before the
element is moved.
Using the suggestion in PR118087, we can define _Temporary_value
similar to the one used in std::vector, so the temporary can be
constructed with uses-allocator construction.
PR libstdc++/118087
libstdc++-v3/ChangeLog:
* include/bits/deque.tcc: Use _Temporary_value in
_M_emplace_aux.
* include/bits/stl_deque.h: Introduce _Temporary_value.
* testsuite/23_containers/deque/modifiers/emplace/118087.cc:
New test.
---
libstdc++-v3/include/bits/deque.tcc | 9 ++-
libstdc++-v3/include/bits/stl_deque.h | 29 +++++++++
.../deque/modifiers/emplace/118087.cc | 62 +++++++++++++++++++
3 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644
libstdc++-v3/testsuite/23_containers/deque/modifiers/emplace/118087.cc
diff --git a/libstdc++-v3/include/bits/deque.tcc
b/libstdc++-v3/include/bits/deque.tcc
index dabb6ec5365..95588ffd860 100644
--- a/libstdc++-v3/include/bits/deque.tcc
+++ b/libstdc++-v3/include/bits/deque.tcc
@@ -664,7 +664,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
deque<_Tp, _Alloc>::
_M_emplace_aux(iterator __pos, _Args&&... __args)
{
- value_type __x_copy(std::forward<_Args>(__args)...); // XXX copy
+ // We should construct this temporary while the deque is
+ // in its current state in case something in __args...
+ // depends on that state before shuffling elements around.
+ _Temporary_value __tmp(this, std::forward<_Args>(__args)...);
#else
typename deque<_Tp, _Alloc>::iterator
deque<_Tp, _Alloc>::
@@ -695,7 +698,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__pos = this->_M_impl._M_start + __index;
_GLIBCXX_MOVE_BACKWARD3(__pos, __back2, __back1);
}
+#if __cplusplus >= 201103L
+ *__pos = _GLIBCXX_MOVE(__tmp._M_val());
This should be std::move(__tmp._M_val())
+#else
*__pos = _GLIBCXX_MOVE(__x_copy);
And this should be just __x_copy.
The _GLIBCXX_MOVE(x) macro expands to std::move(x) in C++11 and later,
and expands to just (x) for C++98. Since you already have a #if check
for precisely the condition "C++11 or later, else C++98" you don't
need the macro.
+#endif
return __pos;
}
diff --git a/libstdc++-v3/include/bits/stl_deque.h
b/libstdc++-v3/include/bits/stl_deque.h
index 7055641ad4e..7cc711efca8 100644
--- a/libstdc++-v3/include/bits/stl_deque.h
+++ b/libstdc++-v3/include/bits/stl_deque.h
@@ -2163,6 +2163,35 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
iterator
_M_insert_aux(iterator __pos, const value_type& __x);
#else
+ struct _Temporary_value
+ {
+ template<typename... _Args>
+ _GLIBCXX20_CONSTEXPR explicit
+ _Temporary_value(deque* __deque, _Args&&... __args) : _M_this(__deque)
+ {
+ _Alloc_traits::construct(_M_this->_M_impl, _M_ptr(),
+ std::forward<_Args>(__args)...);
+ }
+
+ _GLIBCXX20_CONSTEXPR
+ ~_Temporary_value()
+ { _Alloc_traits::destroy(_M_this->_M_impl, _M_ptr()); }
+
+ _GLIBCXX20_CONSTEXPR value_type&
+ _M_val() noexcept { return __tmp_val; }
+
+ private:
+ _GLIBCXX20_CONSTEXPR _Tp*
+ _M_ptr() noexcept { return std::__addressof(__tmp_val); }
+
+ union
+ {
+ _Tp __tmp_val;
+ };
+
+ deque* _M_this;
+ };
+
iterator
_M_insert_aux(iterator __pos, const value_type& __x)
{ return _M_emplace_aux(__pos, __x); }
diff --git
a/libstdc++-v3/testsuite/23_containers/deque/modifiers/emplace/118087.cc
b/libstdc++-v3/testsuite/23_containers/deque/modifiers/emplace/118087.cc
new file mode 100644
index 00000000000..0dc37491824
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/deque/modifiers/emplace/118087.cc
@@ -0,0 +1,62 @@
+// { dg-do run { target c++11 } }
+
+#include <testsuite_hooks.h>
+#include <deque>
+#include <scoped_allocator>
+
+template<typename T>
+struct Alloc
+{
+ Alloc(int i) : id(i) { }
+ template<typename U>
+ Alloc(const Alloc<U>& a) : id(a.id) { }
+
+ using value_type = T;
+ using propagate_on_container_copy_assignment = std::true_type;
+ using propagate_on_container_move_assignment = std::true_type;
+ using propagate_on_container_swap = std::true_type;
+
+ using Base = std::allocator<T>;
+
+ T* allocate(std::size_t n) { return Base().allocate(n); }
+ void deallocate(T* p, std::size_t n) { return Base().deallocate(p, n); }
+
+ int id;
+
+ bool operator==(const Alloc& a) const { return id == a.id; }
+ bool operator!=(const Alloc& a) const { return id != a.id; }
+};
I think this should be simply:
template<typename T>
using Alloc = __gnu_test::propagating_allocator<T, true>;
I defined Alloc<T> by hand in PR 118087 so that the reproducer
wouldn't depend on our testsuite's <util/testsuite_allocator.h> file,
but we can use it in the testsuite.
Otherwise this looks good, thanks.
+
+struct X
+{
+ using allocator_type = Alloc<int>;
+ X() { }
+ X(const X&) { }
+ X(X&&) { }
+ X(const allocator_type& a) : alloc(a) { }
+ X(const X&, const allocator_type& a) : alloc(a) { }
+ X(X&&, const allocator_type& a) : alloc(a) { }
+
+ X& operator=(const X&) = default;
+
+ allocator_type alloc{-1};
+};
+
+int main()
+{
+
+ std::deque<X, std::scoped_allocator_adaptor<Alloc<X>>> d(2,
Alloc<X>(50));
+ VERIFY(d[0].alloc.id == 50);
+ VERIFY(d[1].alloc.id == 50);
+
+ d.emplace(d.begin() + 1);
+ VERIFY(d[1].alloc.id == 50);
+
+ d.emplace_front();
+ VERIFY(d[0].alloc.id == 50);
+
+ d.emplace_back();
+ VERIFY(d[d.size() - 1].alloc.id == 50);
+
+ return 0;
+}
--
2.43.0
From 6c8e01a31be057aab2dd26cccf2f147d6299cb06 Mon Sep 17 00:00:00 2001
From: benwu25 <[email protected]>
Date: Thu, 18 Sep 2025 17:25:41 -0700
Subject: [PATCH] libstdc++: fix element construction in std::deque::emplace
[PR118087]
In order to emplace a value in the middle of a deque, a temporary was
previously constructed directly with __args... in _M_emplace_aux.
This would not work since std::deque is allocator-aware and should
construct elements with _Alloc_traits::construct instead before the
element is moved.
Using the suggestion in PR118087, we can define _Temporary_value
similar to the one used in std::vector, so the temporary can be
constructed with uses-allocator construction.
PR libstdc++/118087
libstdc++-v3/ChangeLog:
* include/bits/deque.tcc: Use _Temporary_value in
_M_emplace_aux.
* include/bits/stl_deque.h: Introduce _Temporary_value.
* testsuite/23_containers/deque/modifiers/emplace/118087.cc:
New test.
---
libstdc++-v3/include/bits/deque.tcc | 9 ++-
libstdc++-v3/include/bits/stl_deque.h | 29 +++++++++
.../deque/modifiers/emplace/118087.cc | 62 +++++++++++++++++++
3 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644
libstdc++-v3/testsuite/23_containers/deque/modifiers/emplace/118087.cc
diff --git a/libstdc++-v3/include/bits/deque.tcc
b/libstdc++-v3/include/bits/deque.tcc
index dabb6ec5365..95588ffd860 100644
--- a/libstdc++-v3/include/bits/deque.tcc
+++ b/libstdc++-v3/include/bits/deque.tcc
@@ -664,7 +664,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
deque<_Tp, _Alloc>::
_M_emplace_aux(iterator __pos, _Args&&... __args)
{
- value_type __x_copy(std::forward<_Args>(__args)...); // XXX copy
+ // We should construct this temporary while the deque is
+ // in its current state in case something in __args...
+ // depends on that state before shuffling elements around.
+ _Temporary_value __tmp(this, std::forward<_Args>(__args)...);
#else
typename deque<_Tp, _Alloc>::iterator
deque<_Tp, _Alloc>::
@@ -695,7 +698,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__pos = this->_M_impl._M_start + __index;
_GLIBCXX_MOVE_BACKWARD3(__pos, __back2, __back1);
}
+#if __cplusplus >= 201103L
+ *__pos = _GLIBCXX_MOVE(__tmp._M_val());
+#else
*__pos = _GLIBCXX_MOVE(__x_copy);
+#endif
return __pos;
}
diff --git a/libstdc++-v3/include/bits/stl_deque.h
b/libstdc++-v3/include/bits/stl_deque.h
index 7055641ad4e..7cc711efca8 100644
--- a/libstdc++-v3/include/bits/stl_deque.h
+++ b/libstdc++-v3/include/bits/stl_deque.h
@@ -2163,6 +2163,35 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
iterator
_M_insert_aux(iterator __pos, const value_type& __x);
#else
+ struct _Temporary_value
+ {
+ template<typename... _Args>
+ _GLIBCXX20_CONSTEXPR explicit
+ _Temporary_value(deque* __deque, _Args&&... __args) : _M_this(__deque)
+ {
+ _Alloc_traits::construct(_M_this->_M_impl, _M_ptr(),
+ std::forward<_Args>(__args)...);
+ }
+
+ _GLIBCXX20_CONSTEXPR
+ ~_Temporary_value()
+ { _Alloc_traits::destroy(_M_this->_M_impl, _M_ptr()); }
+
+ _GLIBCXX20_CONSTEXPR value_type&
+ _M_val() noexcept { return __tmp_val; }
+
+ private:
+ _GLIBCXX20_CONSTEXPR _Tp*
+ _M_ptr() noexcept { return std::__addressof(__tmp_val); }
+
+ union
+ {
+ _Tp __tmp_val;
+ };
+
+ deque* _M_this;
+ };
+
iterator
_M_insert_aux(iterator __pos, const value_type& __x)
{ return _M_emplace_aux(__pos, __x); }
diff --git
a/libstdc++-v3/testsuite/23_containers/deque/modifiers/emplace/118087.cc
b/libstdc++-v3/testsuite/23_containers/deque/modifiers/emplace/118087.cc
new file mode 100644
index 00000000000..0dc37491824
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/deque/modifiers/emplace/118087.cc
@@ -0,0 +1,62 @@
+// { dg-do run { target c++11 } }
+
+#include <testsuite_hooks.h>
+#include <deque>
+#include <scoped_allocator>
+
+template<typename T>
+struct Alloc
+{
+ Alloc(int i) : id(i) { }
+ template<typename U>
+ Alloc(const Alloc<U>& a) : id(a.id) { }
+
+ using value_type = T;
+ using propagate_on_container_copy_assignment = std::true_type;
+ using propagate_on_container_move_assignment = std::true_type;
+ using propagate_on_container_swap = std::true_type;
+
+ using Base = std::allocator<T>;
+
+ T* allocate(std::size_t n) { return Base().allocate(n); }
+ void deallocate(T* p, std::size_t n) { return Base().deallocate(p, n); }
+
+ int id;
+
+ bool operator==(const Alloc& a) const { return id == a.id; }
+ bool operator!=(const Alloc& a) const { return id != a.id; }
+};
+
+struct X
+{
+ using allocator_type = Alloc<int>;
+ X() { }
+ X(const X&) { }
+ X(X&&) { }
+ X(const allocator_type& a) : alloc(a) { }
+ X(const X&, const allocator_type& a) : alloc(a) { }
+ X(X&&, const allocator_type& a) : alloc(a) { }
+
+ X& operator=(const X&) = default;
+
+ allocator_type alloc{-1};
+};
+
+int main()
+{
+
+ std::deque<X, std::scoped_allocator_adaptor<Alloc<X>>> d(2, Alloc<X>(50));
+ VERIFY(d[0].alloc.id == 50);
+ VERIFY(d[1].alloc.id == 50);
+
+ d.emplace(d.begin() + 1);
+ VERIFY(d[1].alloc.id == 50);
+
+ d.emplace_front();
+ VERIFY(d[0].alloc.id == 50);
+
+ d.emplace_back();
+ VERIFY(d[d.size() - 1].alloc.id == 50);
+
+ return 0;
+}
--
2.43.0
