This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: libstdc++ new deque failures
- From: Jonathan Wakely <jwakely at redhat dot com>
- To: Jonathan Wakely <jwakely dot gcc at gmail dot com>
- Cc: David Edelsohn <dje dot gcc at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>
- Date: Mon, 10 Nov 2014 22:50:07 +0000
- Subject: Re: libstdc++ new deque failures
- Authentication-results: sourceware.org; auth=none
- References: <CAGWvnym6DnYVds_5wnmP-MchiCpBRteBeUCT4gz3vNAepJmE+g at mail dot gmail dot com> <CAH6eHdRTh-=_iKoPcf6TZd50BK_VtzV8d3EMxcyKBp7bA9t-KQ at mail dot gmail dot com>
On 05/11/14 17:49 +0000, Jonathan Wakely wrote:
On 5 November 2014 14:14, David Edelsohn wrote:
Jonathan,
I still am seeing new failures in the libstdc++ deque testsuite as of
last night. I don't know if you still are working through the fallout
from the earlier patches, but I wanted to make you aware.
Yes, those tests are meant to fail but I need to adjust the dg-error
line numbers after one of my earlier patches.
I'm working on a patch (I might make other changes to std::deque,
which would require changing the dg-error line numbers yet agan, so
I'm holding off until the other changes are ready ... or I decide not
to make them and just fix the tests.)
Sorry for the noise in the testresults.
Fixed with the attached patch.
The moved-from deque needs to allocate memory for its empty state, but
we don't want to modify the deque until after we know whether that
allocation throws or not. My solution is to make a copy of the
allocator and put that in the moved-from state, then use it to
allocate memory. If that succeeds put the object's own allocator into
the same state and exchange the pointers to transfer ownership.
And these are not related to deque, but appear to be additional issues
in the libstdc++ implementation:
I hadn't seen these ones, I'll take a look, thanks.
I haven't looked at these yet.
commit d3ffebcebad97c71887057f8155f4dbd914f2933
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Mon Nov 10 19:44:23 2014 +0000
Fix std::deque move construction with non-equal allocators.
* include/bits/stl_deque.h (_Deque_base::_Deque_base(_Deque_base&&)):
Dispatch according to whether allocators are always equal.
(_Deque_base::_M_move_impl()): Implement move-from state.
* testsuite/23_containers/deque/requirements/dr438/assign_neg.cc: Fix
dg-error line number.
* testsuite/23_containers/deque/requirements/dr438/
constructor_1_neg.cc: Likewise.
* testsuite/23_containers/deque/requirements/dr438/
constructor_2_neg.cc: Likewise.
* testsuite/23_containers/deque/requirements/dr438/insert_neg.cc:
Likewise.
diff --git a/libstdc++-v3/include/bits/stl_deque.h b/libstdc++-v3/include/bits/stl_deque.h
index d50d3c90..c0052b3 100644
--- a/libstdc++-v3/include/bits/stl_deque.h
+++ b/libstdc++-v3/include/bits/stl_deque.h
@@ -502,14 +502,23 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ /* Caller must initialize map. */ }
#if __cplusplus >= 201103L
- _Deque_base(_Deque_base&& __x)
- : _M_impl(__x._M_get_Tp_allocator())
+ _Deque_base(_Deque_base&& __x, false_type)
+ : _M_impl(__x._M_move_impl())
+ { }
+
+ _Deque_base(_Deque_base&& __x, true_type)
+ : _M_impl(std::move(__x._M_get_Tp_allocator()))
{
_M_initialize_map(0);
if (__x._M_impl._M_map)
this->_M_impl._M_swap_data(__x._M_impl);
}
+ _Deque_base(_Deque_base&& __x)
+ : _Deque_base(std::move(__x),
+ __gnu_cxx::__allocator_always_compares_equal<_Alloc>{})
+ { }
+
_Deque_base(_Deque_base&& __x, const allocator_type& __a, size_type __n)
: _M_impl(__a)
{
@@ -555,18 +564,21 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ }
#if __cplusplus >= 201103L
- _Deque_impl(_Tp_alloc_type&& __a) _GLIBCXX_NOEXCEPT
+ _Deque_impl(_Deque_impl&&) = default;
+
+ _Deque_impl(_Tp_alloc_type&& __a) noexcept
: _Tp_alloc_type(std::move(__a)), _M_map(), _M_map_size(0),
_M_start(), _M_finish()
{ }
#endif
- void _M_swap_data(_Deque_impl& __x)
+ void _M_swap_data(_Deque_impl& __x) _GLIBCXX_NOEXCEPT
{
- std::swap(this->_M_start, __x._M_start);
- std::swap(this->_M_finish, __x._M_finish);
- std::swap(this->_M_map, __x._M_map);
- std::swap(this->_M_map_size, __x._M_map_size);
+ using std::swap;
+ swap(this->_M_start, __x._M_start);
+ swap(this->_M_finish, __x._M_finish);
+ swap(this->_M_map, __x._M_map);
+ swap(this->_M_map_size, __x._M_map_size);
}
};
@@ -618,6 +630,28 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
enum { _S_initial_map_size = 8 };
_Deque_impl _M_impl;
+
+#if __cplusplus >= 201103L
+ private:
+ _Deque_impl
+ _M_move_impl()
+ {
+ if (!_M_impl._M_map)
+ return std::move(_M_impl);
+
+ // Create a copy of the current allocator.
+ _Tp_alloc_type __alloc{_M_get_Tp_allocator()};
+ // Put that copy in a moved-from state.
+ _Tp_alloc_type __unused __attribute((__unused__)) {std::move(__alloc)};
+ // Create an empty map that allocates using the moved-from allocator.
+ _Deque_base __empty{__alloc};
+ // Now safe to modify current allocator and perform non-throwing swaps.
+ _Deque_impl __ret{std::move(_M_get_Tp_allocator())};
+ _M_impl._M_swap_data(__ret);
+ _M_impl._M_swap_data(__empty._M_impl);
+ return __ret;
+ }
+#endif
};
template<typename _Tp, typename _Alloc>
diff --git a/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/assign_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/assign_neg.cc
index 8092ead..b38f3ae 100644
--- a/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/assign_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/assign_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1859 }
+// { dg-error "no matching" "" { target *-*-* } 1881 }
#include <deque>
diff --git a/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/constructor_1_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/constructor_1_neg.cc
index 4abdf46..a30029a 100644
--- a/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/constructor_1_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/constructor_1_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1792 }
+// { dg-error "no matching" "" { target *-*-* } 1814 }
#include <deque>
diff --git a/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/constructor_2_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/constructor_2_neg.cc
index 61bce4e..02eba79 100644
--- a/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/constructor_2_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/constructor_2_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1792 }
+// { dg-error "no matching" "" { target *-*-* } 1814 }
#include <deque>
#include <utility>
diff --git a/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/insert_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/insert_neg.cc
index a0ca00c..8c1dd2e 100644
--- a/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/insert_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/deque/requirements/dr438/insert_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1943 }
+// { dg-error "no matching" "" { target *-*-* } 1965 }
#include <deque>