From patchwork Mon Nov 10 22:50:07 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 409101 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 472A214012C for ; Tue, 11 Nov 2014 09:50:19 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=Ah89UvSU/pXb7V29f zE45kBDv5of0UDIpkVLgZB6aeaT2Ig4C4Ypi77O53nyN0dbaUsucKa0AaMJEwH9k AbCtkA1cKAVWuUUs7Mk32cS2+/xeAQEipepvWPzXRmn5e23g4nCL+Y3SClbZeiIB QOWTUSLB7T3CmnMdcFjFzHMW2s= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=IYsat5wHy1W7gUdSw6VavH5 xPco=; b=YbIifceajOofeZZCkhInkvrfuErvrsxRJsRnr5JWokHtM80EzF5lloq bG8dj53v5iXrNjzh1p8oqP+Pq5xdBU9WTOmFZ2Mw/3gtpdiYgNvYATulM2qqPdAK wsTuwlFnHCdjHAICmJ9Wogi0YaN+npwoM0HVvQ7T7iESdPMmGKlE= Received: (qmail 10847 invoked by alias); 10 Nov 2014 22:50:12 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 10828 invoked by uid 89); 10 Nov 2014 22:50:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 10 Nov 2014 22:50:10 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAAMo8uT026486 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 10 Nov 2014 17:50:08 -0500 Received: from localhost (ovpn-116-136.ams2.redhat.com [10.36.116.136]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sAAMo7BQ013672; Mon, 10 Nov 2014 17:50:08 -0500 Date: Mon, 10 Nov 2014 22:50:07 +0000 From: Jonathan Wakely To: Jonathan Wakely Cc: David Edelsohn , GCC Patches , "libstdc++@gcc.gnu.org" Subject: Re: libstdc++ new deque failures Message-ID: <20141110225007.GK5191@redhat.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) 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 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 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 @@ // . // { dg-do compile } -// { dg-error "no matching" "" { target *-*-* } 1859 } +// { dg-error "no matching" "" { target *-*-* } 1881 } #include 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 @@ // . // { dg-do compile } -// { dg-error "no matching" "" { target *-*-* } 1792 } +// { dg-error "no matching" "" { target *-*-* } 1814 } #include 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 @@ // . // { dg-do compile } -// { dg-error "no matching" "" { target *-*-* } 1792 } +// { dg-error "no matching" "" { target *-*-* } 1814 } #include #include 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 @@ // . // { dg-do compile } -// { dg-error "no matching" "" { target *-*-* } 1943 } +// { dg-error "no matching" "" { target *-*-* } 1965 } #include