From patchwork Wed Jul 8 12:00:42 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 492874 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 E047D140773 for ; Wed, 8 Jul 2015 22:00:55 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=oQN2h07k; dkim-atps=neutral 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:subject:message-id:mime-version:content-type; q=dns; s= default; b=XB2x7ULaoI8CFJUmE7nOCfyE1qDfeek583Qy+n5wbZ8RQCQD495Pw 2oacjUZ4Jqg80VmoYhnJXRean+c1pdaBTpERpPc676MLWh0guz+3XI4HfsE0ZeH+ MQf73/lP82M0yfwH5I/mPUarqByke91HPjzKk6RchjsA2DCwP/H1vE= 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:subject:message-id:mime-version:content-type; s= default; bh=7iSp155X28pbIix8Hjv0s+oLuvY=; b=oQN2h07kfTIjfvkQE6xA RXa5NBkibLiUk1IIeFh8TGoEA9l5bU43jkaCvJZ3d+AJ7Y+hkwCDCGr+37santuT 5ovY3QmV8omGncvBLUg00DpRMOEkGwmzicN/v8k/w5KFUWt/CKvkH9riAtP5J3sa bt9TYOdWHppZt0xy2SKLacw= Received: (qmail 54612 invoked by alias); 8 Jul 2015 12:00:48 -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 54594 invoked by uid 89); 8 Jul 2015 12:00:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=no 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; Wed, 08 Jul 2015 12:00:45 +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 (Postfix) with ESMTPS id 211A6A100C; Wed, 8 Jul 2015 12:00:44 +0000 (UTC) Received: from localhost (ovpn-116-135.ams2.redhat.com [10.36.116.135]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t68C0hlm001643; Wed, 8 Jul 2015 08:00:43 -0400 Date: Wed, 8 Jul 2015 13:00:42 +0100 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [patch] Simplify signatures of std::list members: merge, splice, insert, erase Message-ID: <20150708120042.GM13663@redhat.com> MIME-Version: 1.0 Content-Disposition: inline X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.5.23 (2014-03-12) C++0x changed the signature of std::list::merge to take an rvalue reference, and then LWG DR 1133 [1] added the lvalue reference signature back as an overload. Our implementation follows that history rather literally, as we changed list::merge to take an rvalue in C++0x mode, then later [2] added back an overload taking an lvalue and calling merge(std::move(__x)). It seems to me that it would be simpler to consistently have the lvalue version do all the work, and just make the rvalue one forward to that. That's what patch1.txt does (the same change can be done to debug and profile mode lists). Any objections to this change? patch2.txt then does the same thing for splice(), introducing a __const_iterator typedef [3] to make the signatures consistent for different language modes. I think this makes the code more maintainable, as there are fewer #if blocks with subtly different signatures. Any objections to this one too, including doing the same for debug mode and profile mode? [1] http://cplusplus.github.io/LWG/lwg-defects.html#1133 [2] https://gcc.gnu.org/ml/gcc-patches/2009-12/msg00699.html [3] https://gcc.gnu.org/ml/libstdc++/2014-08/msg00151.html commit d37a4ae4a9e4883978f1817b01995c2608624070 Author: Jonathan Wakely Date: Wed Jul 8 12:15:36 2015 +0100 * include/bits/list.tcc (list::merge): Always define lvalue overload. * include/bits/stl_list.h (list::merge): Make rvalue overload forward to lvalue overload. commit 682940254cdb55b30b76ce28d3910897bba6673a Author: Jonathan Wakely Date: Wed Jul 8 12:50:10 2015 +0100 * include/bits/list.tcc (list::insert, list::erase): Use __const_iterator to make declarations consistent for C++98 and C++11. * include/bits/stl_list.h (list::__const_iterator): Define. (list::insert, list::erase): Use __const_iterator to make declarations consistent for C++98 and C++11. (list::splice): Likewise, and forward from the overloads taking lvalues to the overloads taking rvalues, instead of vice versa. diff --git a/libstdc++-v3/include/bits/list.tcc b/libstdc++-v3/include/bits/list.tcc index 4b8418e..7421e02 100644 --- a/libstdc++-v3/include/bits/list.tcc +++ b/libstdc++-v3/include/bits/list.tcc @@ -98,11 +98,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER template typename list<_Tp, _Alloc>::iterator list<_Tp, _Alloc>:: -#if __cplusplus >= 201103L - insert(const_iterator __position, const value_type& __x) -#else - insert(iterator __position, const value_type& __x) -#endif + insert(__const_iterator __position, const value_type& __x) { _Node* __tmp = _M_create_node(__x); __tmp->_M_hook(__position._M_const_cast()._M_node); @@ -147,11 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER template typename list<_Tp, _Alloc>::iterator list<_Tp, _Alloc>:: -#if __cplusplus >= 201103L - erase(const_iterator __position) noexcept -#else - erase(iterator __position) -#endif + erase(__const_iterator __position) _GLIBCXX_NOEXCEPT { iterator __ret = iterator(__position._M_node->_M_next); _M_erase(__position._M_const_cast()); diff --git a/libstdc++-v3/include/bits/stl_list.h b/libstdc++-v3/include/bits/stl_list.h index d0f0fd6..41a6767 100644 --- a/libstdc++-v3/include/bits/stl_list.h +++ b/libstdc++-v3/include/bits/stl_list.h @@ -537,6 +537,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 using _Base::_M_get_node; using _Base::_M_get_Node_allocator; + // type used for positions in insert, erase etc. +#if __cplusplus < 201103L + typedef iterator __const_iterator; +#else + typedef const_iterator __const_iterator; +#endif + /** * @param __args An instance of user data. * @@ -1139,23 +1146,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 template iterator emplace(const_iterator __position, _Args&&... __args); +#endif /** * @brief Inserts given value into %list before specified iterator. - * @param __position A const_iterator into the %list. - * @param __x Data to be inserted. - * @return An iterator that points to the inserted data. - * - * This function will insert a copy of the given value before - * the specified location. Due to the nature of a %list this - * operation can be done in constant time, and does not - * invalidate iterators and references. - */ - iterator - insert(const_iterator __position, const value_type& __x); -#else - /** - * @brief Inserts given value into %list before specified iterator. * @param __position An iterator into the %list. * @param __x Data to be inserted. * @return An iterator that points to the inserted data. @@ -1166,8 +1160,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * invalidate iterators and references. */ iterator - insert(iterator __position, const value_type& __x); -#endif + insert(__const_iterator __position, const value_type& __x); #if __cplusplus >= 201103L /** @@ -1205,24 +1198,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 { return this->insert(__p, __l.begin(), __l.end()); } #endif -#if __cplusplus >= 201103L - /** - * @brief Inserts a number of copies of given data into the %list. - * @param __position A const_iterator into the %list. - * @param __n Number of elements to be inserted. - * @param __x Data to be inserted. - * @return An iterator pointing to the first element inserted - * (or __position). - * - * This function will insert a specified number of copies of the - * given data before the location specified by @a position. - * - * This operation is linear in the number of elements inserted and - * does not invalidate iterators and references. - */ - iterator - insert(const_iterator __position, size_type __n, const value_type& __x); -#else /** * @brief Inserts a number of copies of given data into the %list. * @param __position An iterator into the %list. @@ -1236,12 +1211,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * does not invalidate iterators and references. */ void - insert(iterator __position, size_type __n, const value_type& __x) + insert(__const_iterator __position, size_type __n, const value_type& __x) { list __tmp(__n, __x, get_allocator()); splice(__position, __tmp); } -#endif #if __cplusplus >= 201103L /** @@ -1304,11 +1278,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * any way. Managing the pointer is the user's responsibility. */ iterator -#if __cplusplus >= 201103L - erase(const_iterator __position) noexcept; -#else - erase(iterator __position); -#endif + erase(__const_iterator __position) _GLIBCXX_NOEXCEPT; /** * @brief Remove a range of elements. @@ -1329,11 +1299,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * is the user's responsibility. */ iterator -#if __cplusplus >= 201103L - erase(const_iterator __first, const_iterator __last) noexcept -#else - erase(iterator __first, iterator __last) -#endif + erase(__const_iterator __first, __const_iterator __last) + _GLIBCXX_NOEXCEPT { while (__first != __last) __first = erase(__first); @@ -1392,11 +1359,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * Requires this != @a __x. */ void -#if __cplusplus >= 201103L - splice(const_iterator __position, list&& __x) noexcept -#else - splice(iterator __position, list& __x) -#endif + splice(__const_iterator __position, list& __x) _GLIBCXX_NOEXCEPT { if (!__x.empty()) { @@ -1412,24 +1375,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 #if __cplusplus >= 201103L void - splice(const_iterator __position, list& __x) noexcept - { splice(__position, std::move(__x)); } + splice(const_iterator __position, list&& __x) noexcept + { splice(__position, __x); } #endif -#if __cplusplus >= 201103L - /** - * @brief Insert element from another %list. - * @param __position Const_iterator referencing the element to - * insert before. - * @param __x Source list. - * @param __i Const_iterator referencing the element to move. - * - * Removes the element in list @a __x referenced by @a __i and - * inserts it into the current list before @a __position. - */ - void - splice(const_iterator __position, list&& __x, const_iterator __i) noexcept -#else /** * @brief Insert element from another %list. * @param __position Iterator referencing the element to insert before. @@ -1440,8 +1389,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * inserts it into the current list before @a __position. */ void - splice(iterator __position, list& __x, iterator __i) -#endif + splice(__const_iterator __position, list& __x, __const_iterator __i) + _GLIBCXX_USE_NOEXCEPT { iterator __j = __i._M_const_cast(); ++__j; @@ -1470,28 +1419,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * inserts it into the current list before @a __position. */ void - splice(const_iterator __position, list& __x, const_iterator __i) noexcept - { splice(__position, std::move(__x), __i); } + splice(const_iterator __position, list&& __x, const_iterator __i) + noexcept + { splice(__position, __x, __i); } #endif -#if __cplusplus >= 201103L - /** - * @brief Insert range from another %list. - * @param __position Const_iterator referencing the element to - * insert before. - * @param __x Source list. - * @param __first Const_iterator referencing the start of range in x. - * @param __last Const_iterator referencing the end of range in x. - * - * Removes elements in the range [__first,__last) and inserts them - * before @a __position in constant time. - * - * Undefined if @a __position is in [__first,__last). - */ - void - splice(const_iterator __position, list&& __x, const_iterator __first, - const_iterator __last) noexcept -#else /** * @brief Insert range from another %list. * @param __position Iterator referencing the element to insert before. @@ -1505,9 +1437,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * Undefined if @a __position is in [__first,__last). */ void - splice(iterator __position, list& __x, iterator __first, - iterator __last) -#endif + splice(__const_iterator __position, list& __x, __const_iterator __first, + __const_iterator __last) _GLIBCXX_NOEXCEPT { if (__first != __last) { @@ -1539,9 +1470,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * Undefined if @a __position is in [__first,__last). */ void - splice(const_iterator __position, list& __x, const_iterator __first, + splice(const_iterator __position, list&& __x, const_iterator __first, const_iterator __last) noexcept - { splice(__position, std::move(__x), __first, __last); } + { splice(__position, __x, __first, __last); } #endif /** diff --git a/libstdc++-v3/include/bits/list.tcc b/libstdc++-v3/include/bits/list.tcc index 714d9b5..4b8418e 100644 --- a/libstdc++-v3/include/bits/list.tcc +++ b/libstdc++-v3/include/bits/list.tcc @@ -370,11 +370,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER template void list<_Tp, _Alloc>:: -#if __cplusplus >= 201103L - merge(list&& __x) -#else merge(list& __x) -#endif { // _GLIBCXX_RESOLVE_LIB_DEFECTS // 300. list::merge() specification incomplete @@ -407,11 +403,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER template void list<_Tp, _Alloc>:: -#if __cplusplus >= 201103L - merge(list&& __x, _StrictWeakOrdering __comp) -#else merge(list& __x, _StrictWeakOrdering __comp) -#endif { // _GLIBCXX_RESOLVE_LIB_DEFECTS // 300. list::merge() specification incomplete diff --git a/libstdc++-v3/include/bits/stl_list.h b/libstdc++-v3/include/bits/stl_list.h index 409f1fc..d0f0fd6 100644 --- a/libstdc++-v3/include/bits/stl_list.h +++ b/libstdc++-v3/include/bits/stl_list.h @@ -1611,16 +1611,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * sorted order, leaving @a __x empty when complete. Elements in * this list precede elements in @a __x that are equal. */ -#if __cplusplus >= 201103L void - merge(list&& __x); + merge(list&); +#if __cplusplus >= 201103L void - merge(list& __x) - { merge(std::move(__x)); } -#else - void - merge(list& __x); + merge(list&& __x) + { merge(__x); } #endif /** @@ -1636,19 +1633,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * in this list precede elements in @a __x that are equivalent * according to StrictWeakOrdering(). */ -#if __cplusplus >= 201103L template void - merge(list&& __x, _StrictWeakOrdering __comp); + merge(list&, _StrictWeakOrdering); +#if __cplusplus >= 201103L template void - merge(list& __x, _StrictWeakOrdering __comp) - { merge(std::move(__x), __comp); } -#else - template - void - merge(list& __x, _StrictWeakOrdering __comp); + merge(list&& __x, _StrictWeakOrdering __comp) + { merge(__x, __comp); } #endif /**