From patchwork Tue Jun 7 16:19:50 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Carlini X-Patchwork-Id: 99288 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 ADA3EB6FB4 for ; Wed, 8 Jun 2011 02:20:03 +1000 (EST) Received: (qmail 21279 invoked by alias); 7 Jun 2011 16:19:55 -0000 Received: (qmail 21263 invoked by uid 22791); 7 Jun 2011 16:19:51 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from smtp205.alice.it (HELO smtp205.alice.it) (82.57.200.101) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 07 Jun 2011 16:19:24 +0000 Received: from [192.168.1.4] (79.52.211.22) by smtp205.alice.it (8.5.124.08) id 4DE634750089F43E; Tue, 7 Jun 2011 18:19:20 +0200 Message-ID: <4DEE4FA6.7040109@oracle.com> Date: Tue, 07 Jun 2011 18:19:50 +0200 From: Paolo Carlini User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110414 SUSE/3.1.10 Thunderbird/3.1.10 MIME-Version: 1.0 To: "gcc-patches@gcc.gnu.org" CC: libstdc++ Subject: [v3] Use move_if_noexcept in std::vector X-IsSubscribed: yes 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 Hi, the below starts using it, for correctness wrt move constructors which can throw. For sure we are missing the optimization which uses the default-constructor + swap when the type isn't nothrow move constructible, per Note 5 in n3050, for example. Tested x86_64-linux, committed. Paolo. ///////////////////// 2011-06-07 Paolo Carlini * include/bits/move.h (struct __move_if_noexcept_cond): Add. (move_if_noexcept): Use the latter. * include/bits/stl_iterator.h (__make_move_if_noexcept_iterator, _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR): Add. * include/bits/stl_uninitialized.h (__uninitialized_move_if_noexcept_a): Add. * include/bits/vector.tcc (vector<>::reserve): Use _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR. (vector<>::_M_insert_aux, _M_fill_insert, _M_default_append, _M_range_insert): Use __uninitialized_move_if_noexcept_a. * testsuite/util/testsuite_rvalref.h (throwing_move_constructor): Add. (copycounter::copycounter(copycounter&&)): Use noexcept. * testsuite/23_containers/vector/modifiers/moveable2.cc: New. * testsuite/23_containers/vector/capacity/resize/moveable2.cc: Likewise. * testsuite/23_containers/vector/capacity/reserve/moveable2.cc: Likewise. Index: include/bits/move.h =================================================================== --- include/bits/move.h (revision 174739) +++ include/bits/move.h (working copy) @@ -82,6 +82,12 @@ move(_Tp&& __t) noexcept { return static_cast::type&&>(__t); } + + template + struct __move_if_noexcept_cond + : public __and_<__not_>, + is_copy_constructible<_Tp>>::type { }; + /** * @brief Move unless it could throw and the type is copyable. * @ingroup utilities @@ -90,9 +96,7 @@ */ template inline typename - conditional<__and_<__not_>, - is_copy_constructible<_Tp>>::value, - const _Tp&, _Tp&&>::type + conditional<__move_if_noexcept_cond<_Tp>::value, const _Tp&, _Tp&&>::type move_if_noexcept(_Tp& __x) noexcept { return std::move(__x); } Index: include/bits/stl_uninitialized.h =================================================================== --- include/bits/stl_uninitialized.h (revision 174739) +++ include/bits/stl_uninitialized.h (working copy) @@ -269,6 +269,19 @@ __result, __alloc); } + template + inline _ForwardIterator + __uninitialized_move_if_noexcept_a(_InputIterator __first, + _InputIterator __last, + _ForwardIterator __result, + _Allocator& __alloc) + { + return std::__uninitialized_copy_a + (_GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__first), + _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__last), __result, __alloc); + } + template void __uninitialized_fill_a(_ForwardIterator __first, _ForwardIterator __last, Index: include/bits/stl_iterator.h =================================================================== --- include/bits/stl_iterator.h (revision 174739) +++ include/bits/stl_iterator.h (working copy) @@ -1118,14 +1118,25 @@ make_move_iterator(const _Iterator& __i) { return move_iterator<_Iterator>(__i); } + template::value_type>::value, + _Iterator, move_iterator<_Iterator>>::type> + inline _ReturnType + __make_move_if_noexcept_iterator(_Iterator __i) + { return _ReturnType(__i); } + // @} group iterators _GLIBCXX_END_NAMESPACE_VERSION } // namespace #define _GLIBCXX_MAKE_MOVE_ITERATOR(_Iter) std::make_move_iterator(_Iter) +#define _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(_Iter) \ + std::__make_move_if_noexcept_iterator(_Iter) #else #define _GLIBCXX_MAKE_MOVE_ITERATOR(_Iter) (_Iter) +#define _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(_Iter) (_Iter) #endif // __GXX_EXPERIMENTAL_CXX0X__ #endif Index: include/bits/vector.tcc =================================================================== --- include/bits/vector.tcc (revision 174739) +++ include/bits/vector.tcc (working copy) @@ -72,8 +72,8 @@ { const size_type __old_size = size(); pointer __tmp = _M_allocate_and_copy(__n, - _GLIBCXX_MAKE_MOVE_ITERATOR(this->_M_impl._M_start), - _GLIBCXX_MAKE_MOVE_ITERATOR(this->_M_impl._M_finish)); + _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(this->_M_impl._M_start), + _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(this->_M_impl._M_finish)); std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish, _M_get_Tp_allocator()); _M_deallocate(this->_M_impl._M_start, @@ -337,17 +337,17 @@ #endif __new_finish = 0; - __new_finish = - std::__uninitialized_move_a(this->_M_impl._M_start, - __position.base(), __new_start, - _M_get_Tp_allocator()); + __new_finish + = std::__uninitialized_move_if_noexcept_a + (this->_M_impl._M_start, __position.base(), + __new_start, _M_get_Tp_allocator()); + ++__new_finish; - __new_finish = - std::__uninitialized_move_a(__position.base(), - this->_M_impl._M_finish, - __new_finish, - _M_get_Tp_allocator()); + __new_finish + = std::__uninitialized_move_if_noexcept_a + (__position.base(), this->_M_impl._M_finish, + __new_finish, _M_get_Tp_allocator()); } __catch(...) { @@ -423,18 +423,17 @@ _M_get_Tp_allocator()); __new_finish = 0; - __new_finish = - std::__uninitialized_move_a(this->_M_impl._M_start, - __position.base(), - __new_start, - _M_get_Tp_allocator()); + __new_finish + = std::__uninitialized_move_if_noexcept_a + (this->_M_impl._M_start, __position.base(), + __new_start, _M_get_Tp_allocator()); + __new_finish += __n; - __new_finish = - std::__uninitialized_move_a(__position.base(), - this->_M_impl._M_finish, - __new_finish, - _M_get_Tp_allocator()); + __new_finish + = std::__uninitialized_move_if_noexcept_a + (__position.base(), this->_M_impl._M_finish, + __new_finish, _M_get_Tp_allocator()); } __catch(...) { @@ -484,11 +483,10 @@ pointer __new_finish(__new_start); __try { - __new_finish = - std::__uninitialized_move_a(this->_M_impl._M_start, - this->_M_impl._M_finish, - __new_start, - _M_get_Tp_allocator()); + __new_finish + = std::__uninitialized_move_if_noexcept_a + (this->_M_impl._M_start, this->_M_impl._M_finish, + __new_start, _M_get_Tp_allocator()); std::__uninitialized_default_n_a(__new_finish, __n, _M_get_Tp_allocator()); __new_finish += __n; @@ -577,20 +575,18 @@ pointer __new_finish(__new_start); __try { - __new_finish = - std::__uninitialized_move_a(this->_M_impl._M_start, - __position.base(), - __new_start, - _M_get_Tp_allocator()); - __new_finish = - std::__uninitialized_copy_a(__first, __last, - __new_finish, - _M_get_Tp_allocator()); - __new_finish = - std::__uninitialized_move_a(__position.base(), - this->_M_impl._M_finish, - __new_finish, - _M_get_Tp_allocator()); + __new_finish + = std::__uninitialized_move_if_noexcept_a + (this->_M_impl._M_start, __position.base(), + __new_start, _M_get_Tp_allocator()); + __new_finish + = std::__uninitialized_copy_a(__first, __last, + __new_finish, + _M_get_Tp_allocator()); + __new_finish + = std::__uninitialized_move_if_noexcept_a + (__position.base(), this->_M_impl._M_finish, + __new_finish, _M_get_Tp_allocator()); } __catch(...) { Index: testsuite/23_containers/vector/modifiers/moveable2.cc =================================================================== --- testsuite/23_containers/vector/modifiers/moveable2.cc (revision 0) +++ testsuite/23_containers/vector/modifiers/moveable2.cc (revision 0) @@ -0,0 +1,77 @@ +// { dg-options "-std=gnu++0x" } + +// 2011-06-07 Paolo Carlini + +// Copyright (C) 2011 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 +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +#include +#include +#include + +void +test01() +{ + bool test __attribute__((unused)) = true; + using namespace __gnu_test; + + std::vector v1; + + throwing_move_constructor tmc; + + v1.push_back(tmc); + VERIFY( v1.size() == 1 ); + + v1.push_back(tmc); + VERIFY( v1.size() == 2 ); + + v1.insert(v1.end(), tmc); + VERIFY( v1.size() == 3 ); + + v1.insert(v1.end(), 100, tmc); + VERIFY( v1.size() == 103 ); + + v1.insert(v1.end(), 10, tmc); + VERIFY( v1.size() == 113 ); + + v1.insert(v1.end(), 1, tmc); + VERIFY( v1.size() == 114 ); + + std::vector v2; + + throwing_move_constructor tmca[] + = { throwing_move_constructor(), throwing_move_constructor(), + throwing_move_constructor(), throwing_move_constructor() }; + + v2.insert(v2.end(), tmca, tmca + 1); + VERIFY( v2.size() == 1 ); + + v2.insert(v2.end(), tmca, tmca + 4); + VERIFY( v2.size() == 5 ); + + v2.insert(v2.end(), tmca, tmca + 2); + VERIFY( v2.size() == 7 ); + + v2.insert(v2.end(), tmca, tmca + 1); + VERIFY( v2.size() == 8 ); +} + +int main() +{ + test01(); + return 0; +} Index: testsuite/23_containers/vector/capacity/resize/moveable2.cc =================================================================== --- testsuite/23_containers/vector/capacity/resize/moveable2.cc (revision 0) +++ testsuite/23_containers/vector/capacity/resize/moveable2.cc (revision 0) @@ -0,0 +1,57 @@ +// { dg-options "-std=gnu++0x" } + +// 2011-06-07 Paolo Carlini + +// Copyright (C) 2011 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 +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +#include +#include +#include + +void +test01() +{ + bool test __attribute__((unused)) = true; + using namespace __gnu_test; + + std::vector v(5); + + v.resize(50); + VERIFY( v.size() == 50 ); + + v.reserve(200); + VERIFY( v.capacity() >= 200 ); + + v.resize(100); + VERIFY( v.size() == 100 ); + + v.resize(500, throwing_move_constructor()); + VERIFY( v.size() == 500 ); + + v.reserve(2000); + VERIFY( v.capacity() >= 2000 ); + + v.resize(1000, throwing_move_constructor()); + VERIFY( v.size() == 1000 ); +} + +int main() +{ + test01(); + return 0; +} Index: testsuite/23_containers/vector/capacity/reserve/moveable2.cc =================================================================== --- testsuite/23_containers/vector/capacity/reserve/moveable2.cc (revision 0) +++ testsuite/23_containers/vector/capacity/reserve/moveable2.cc (revision 0) @@ -0,0 +1,45 @@ +// { dg-options "-std=gnu++0x" } + +// 2011-06-07 Paolo Carlini + +// Copyright (C) 2011 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 +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +#include +#include +#include + +void +test01() +{ + bool test __attribute__((unused)) = true; + using namespace __gnu_test; + + std::vector v(5); + + v.reserve(50); + VERIFY( v.capacity() >= 50 ); + + v.reserve(500); + VERIFY( v.capacity() >= 500 ); +} + +int main() +{ + test01(); + return 0; +} Index: testsuite/util/testsuite_rvalref.h =================================================================== --- testsuite/util/testsuite_rvalref.h (revision 174735) +++ testsuite/util/testsuite_rvalref.h (working copy) @@ -117,7 +117,7 @@ ++copycount; } - copycounter(copycounter&& in) + copycounter(copycounter&& in) noexcept { bool test __attribute__((unused)) = true; VERIFY( in.valid == true ); @@ -156,7 +156,7 @@ return *this; } - ~copycounter() + ~copycounter() noexcept { valid = false; } }; @@ -246,6 +246,16 @@ return lh.val < rh.val; } + struct throwing_move_constructor + { + throwing_move_constructor() = default; + + throwing_move_constructor(throwing_move_constructor&&) + { throw 1; } + + throwing_move_constructor(const throwing_move_constructor&) = default; + }; + } // namespace __gnu_test namespace std