From patchwork Mon Nov 5 21:13:34 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 197308 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]) by ozlabs.org (Postfix) with SMTP id 665502C00BE for ; Tue, 6 Nov 2012 08:14:12 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1352754853; h=Comment: DomainKey-Signature:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=fMmwg45kFstIcknRnJaQUvpAblE=; b=w3jNCNYPcJrG+Ip OWMOPRpRz4AramorsG+S+4SQNcm0WAQmiZnth3kd9xT0SfE6lW6ckyFBjV7ISzxh M+wOuIU6JpmgzhivXFD0c4YFJ8agzGD6wKSOc0BihiLSeANzKNJENYT4p358Fco3 cWyiKo+DFo0IvgoFzeQ+IRzKeSJ4= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=ZYZ1tt1FiyAMa6d+iZrxRt9K6RjoN1pEOmV/awoR1SUIG5La1ADi2JcEffykbi R600sFnQuS1S1qr5IWdYASl+VMnb6AiUjr7L4p0iTlCAMIfbzV6Y+0ka183i0BGf GpvKpdvQb1+4y7wFeA841TgeCl7IFSITFcW0DGcB+ZDes=; Received: (qmail 7272 invoked by alias); 5 Nov 2012 21:13:42 -0000 Received: (qmail 7094 invoked by uid 22791); 5 Nov 2012 21:13:41 -0000 X-SWARE-Spam-Status: No, hits=-5.3 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, KHOP_RCVD_TRUST, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_NO, RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-ie0-f175.google.com (HELO mail-ie0-f175.google.com) (209.85.223.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 05 Nov 2012 21:13:35 +0000 Received: by mail-ie0-f175.google.com with SMTP id c13so9387434ieb.20 for ; Mon, 05 Nov 2012 13:13:34 -0800 (PST) MIME-Version: 1.0 Received: by 10.50.169.100 with SMTP id ad4mr10780235igc.50.1352150014663; Mon, 05 Nov 2012 13:13:34 -0800 (PST) Received: by 10.42.158.202 with HTTP; Mon, 5 Nov 2012 13:13:34 -0800 (PST) In-Reply-To: References: Date: Mon, 5 Nov 2012 21:13:34 +0000 Message-ID: Subject: Re: [v3] Fix allocator-aware container requirements for forward_list From: Jonathan Wakely To: "libstdc++" , gcc-patches 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 This updates the debug-mode and profile-mode forward_list code to match my recent allocator changes to the default mode. It also adds checking for unswappable allocators, and fixes some tests that those new checks showed were broken. * include/profile/forward_list: Update to meet allocator-aware requirements. * include/debug/forward_list: Likewise. * include/debug/vector: Verify allocators are swapped or equal. * include/debug/macros.h (__glibcxx_check_equal_allocs): Define. * include/debug/formatter.h: Add new debug message. * src/c++11/debug.cc: Likewise. * testsuite/23_containers/forward_list/allocator/swap.cc: Do not swap containers with non-propagating, non-equal allocators. * testsuite/23_containers/vector/allocator/swap.cc: Likewise. Tested x86_64-linux, committed to trunk. commit e537dbf92d79d68a4ff2f15864183da1cf17c821 Author: Jonathan Wakely Date: Mon Nov 5 00:15:12 2012 +0000 * include/profile/forward_list: Update to meet allocator-aware requirements. * include/debug/forward_list: Likewise. * include/debug/vector: Verify allocators are swapped or equal. * include/debug/macros.h (__glibcxx_check_equal_allocs): Define. * include/debug/formatter.h: Add new debug message. * src/c++11/debug.cc: Likewise. * testsuite/23_containers/forward_list/allocator/swap.cc: Do not swap containers with non-propagating, non-equal allocators. * testsuite/23_containers/vector/allocator/swap.cc: Likewise. diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h index 1d29d8c..d622ed1 100644 --- a/libstdc++-v3/include/debug/formatter.h +++ b/libstdc++-v3/include/debug/formatter.h @@ -114,7 +114,8 @@ namespace __gnu_debug __msg_self_move_assign, // unordered container buckets __msg_bucket_index_oob, - __msg_valid_load_factor + __msg_valid_load_factor, + __msg_equal_allocs }; class _Error_formatter diff --git a/libstdc++-v3/include/debug/forward_list b/libstdc++-v3/include/debug/forward_list index 8ad4336..61ae6ed 100644 --- a/libstdc++-v3/include/debug/forward_list +++ b/libstdc++-v3/include/debug/forward_list @@ -49,6 +49,12 @@ namespace __debug typedef typename _Base::iterator _Base_iterator; typedef typename _Base::const_iterator _Base_const_iterator; + + typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template + rebind<_GLIBCXX_STD_C::_Fwd_list_node<_Tp>>::other _Node_alloc_type; + + typedef __gnu_cxx::__alloc_traits<_Node_alloc_type> _Node_alloc_traits; + public: typedef typename _Base::reference reference; typedef typename _Base::const_reference const_reference; @@ -78,12 +84,15 @@ namespace __debug forward_list(forward_list&& __list, const _Alloc& __al) : _Base(std::move(__list._M_base()), __al) { - this->_M_swap(__list); + if (__list.get_allocator() == __al) + this->_M_swap(__list); + else + __list._M_invalidate_all(); } explicit - forward_list(size_type __n) - : _Base(__n) + forward_list(size_type __n, const _Alloc& __al = _Alloc()) + : _Base(__n, __al) { } forward_list(size_type __n, const _Tp& __value, @@ -128,12 +137,17 @@ namespace __debug forward_list& operator=(forward_list&& __list) + noexcept(_Node_alloc_traits::_S_nothrow_move()) { - // NB: DR 1204. - // NB: DR 675. __glibcxx_check_self_move_assign(__list); - clear(); - swap(__list); + bool xfer_memory = _Node_alloc_traits::_S_propagate_on_move_assign() + || __list.get_allocator() == this->get_allocator(); + static_cast<_Base&>(*this) = std::move(__list); + if (xfer_memory) + this->_M_swap(__list); + else + this->_M_invalidate_all(); + __list._M_invalidate_all(); return *this; } @@ -333,7 +347,10 @@ namespace __debug void swap(forward_list& __list) + noexcept(_Node_alloc_traits::_S_nothrow_swap()) { + if (!_Node_alloc_traits::_S_propagate_on_swap()) + __glibcxx_check_equal_allocs(__list); _Base::swap(__list); this->_M_swap(__list); } diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h index 3df0c9b..30606d5 100644 --- a/libstdc++-v3/include/debug/macros.h +++ b/libstdc++-v3/include/debug/macros.h @@ -333,6 +333,11 @@ _GLIBCXX_DEBUG_VERIFY(_F > 0.0f, \ _M_message(__gnu_debug::__msg_valid_load_factor) \ ._M_sequence(*this, "this")) +#define __glibcxx_check_equal_allocs(_Other) \ +_GLIBCXX_DEBUG_VERIFY(this->get_allocator() == _Other.get_allocator(), \ + _M_message(__gnu_debug::__msg_equal_allocs) \ + ._M_sequence(*this, "this")) + #ifdef _GLIBCXX_DEBUG_PEDANTIC # define __glibcxx_check_string(_String) _GLIBCXX_DEBUG_ASSERT(_String != 0) # define __glibcxx_check_string_len(_String,_Len) \ diff --git a/libstdc++-v3/include/debug/vector b/libstdc++-v3/include/debug/vector index 9e0f843..9c33fdf 100644 --- a/libstdc++-v3/include/debug/vector +++ b/libstdc++-v3/include/debug/vector @@ -550,6 +550,10 @@ namespace __debug noexcept(_Alloc_traits::_S_nothrow_swap()) #endif { +#ifdef __GXX_EXPERIMENTAL_CXX0X__ + if (!_Alloc_traits::_S_propagate_on_swap()) + __glibcxx_check_equal_allocs(__x); +#endif _Base::swap(__x); this->_M_swap(__x); std::swap(_M_guaranteed_capacity, __x._M_guaranteed_capacity); diff --git a/libstdc++-v3/include/profile/forward_list b/libstdc++-v3/include/profile/forward_list index 618b248..a44ea7a 100644 --- a/libstdc++-v3/include/profile/forward_list +++ b/libstdc++-v3/include/profile/forward_list @@ -1,6 +1,6 @@ // -*- C++ -*- -// Copyright (C) 2010, 2011 Free Software Foundation, Inc. +// Copyright (C) 2010-2012 Free Software Foundation, Inc. // // This file is part of the GNU ISO C++ Library. This library is free // software; you can redistribute it and/or modify it under the @@ -46,10 +46,14 @@ namespace __profile { typedef _GLIBCXX_STD_C::forward_list<_Tp, _Alloc> _Base; + typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template + rebind<_GLIBCXX_STD_C::_Fwd_list_node<_Tp>>::other _Node_alloc_type; + + typedef __gnu_cxx::__alloc_traits<_Node_alloc_type> _Node_alloc_traits; + public: typedef typename _Base::size_type size_type; - public: // 23.2.3.1 construct/copy/destroy: explicit forward_list(const _Alloc& __al = _Alloc()) @@ -64,8 +68,8 @@ namespace __profile { } explicit - forward_list(size_type __n) - : _Base(__n) + forward_list(size_type __n, const _Alloc& __al = _Alloc()) + : _Base(__n, __al) { } forward_list(size_type __n, const _Tp& __value, @@ -103,11 +107,9 @@ namespace __profile forward_list& operator=(forward_list&& __list) + noexcept(_Node_alloc_traits::_S_nothrow_move()) { - // NB: DR 1204. - // NB: DR 675. - _Base::clear(); - _Base::swap(__list); + static_cast<_Base&>(*this) = std::move(__list); return *this; } diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc index 8a18026..f7725ed 100644 --- a/libstdc++-v3/src/c++11/debug.cc +++ b/libstdc++-v3/src/c++11/debug.cc @@ -181,7 +181,8 @@ namespace __gnu_debug "attempt to self move assign", "attempt to access container with out-of-bounds bucket index %2;," " container only holds %3; buckets", - "load factor shall be positive" + "load factor shall be positive", + "allocators must be equal" }; void diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/allocator/swap.cc b/libstdc++-v3/testsuite/23_containers/forward_list/allocator/swap.cc index 60d83d4..1d1e217 100644 --- a/libstdc++-v3/testsuite/23_containers/forward_list/allocator/swap.cc +++ b/libstdc++-v3/testsuite/23_containers/forward_list/allocator/swap.cc @@ -25,6 +25,23 @@ struct T { int i; }; using __gnu_test::propagating_allocator; +// It is undefined behaviour to swap() containers wth unequal allocators +// if the allocator doesn't propagate, so ensure the allocators compare +// equal, while still being able to test propagation via get_personality(). +bool +operator==(const propagating_allocator&, + const propagating_allocator&) +{ + return true; +} + +bool +operator!=(const propagating_allocator&, + const propagating_allocator&) +{ + return false; +} + void test01() { bool test __attribute__((unused)) = true; diff --git a/libstdc++-v3/testsuite/23_containers/vector/allocator/swap.cc b/libstdc++-v3/testsuite/23_containers/vector/allocator/swap.cc index 808753e..2ca19db 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/allocator/swap.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/allocator/swap.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Free Software Foundation +// Copyright (C) 2011-2012 Free Software Foundation // // This file is part of the GNU ISO C++ Library. This library is free // software; you can redistribute it and/or modify it under the @@ -25,6 +25,23 @@ struct T { int i; }; using __gnu_test::propagating_allocator; +// It is undefined behaviour to swap() containers wth unequal allocators +// if the allocator doesn't propagate, so ensure the allocators compare +// equal, while still being able to test propagation via get_personality(). +bool +operator==(const propagating_allocator&, + const propagating_allocator&) +{ + return true; +} + +bool +operator!=(const propagating_allocator&, + const propagating_allocator&) +{ + return false; +} + void test01() { bool test __attribute__((unused)) = true;