From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 90424 invoked by alias); 28 Oct 2016 19:43:05 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 90374 invoked by uid 89); 28 Oct 2016 19:43:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=qualification, guaranty, franois X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-wm0-f43.google.com Received: from mail-wm0-f43.google.com (HELO mail-wm0-f43.google.com) (74.125.82.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 28 Oct 2016 19:42:58 +0000 Received: by mail-wm0-f43.google.com with SMTP id e69so123590386wmg.0; Fri, 28 Oct 2016 12:42:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:to:from:subject:message-id:date:user-agent :mime-version; bh=mssNbqdNCmvV3mbUylXkPjrcKWwAuhlj1107j5ucdTg=; b=SQ6cbTzMGS/iYFRNGiOm/uj7AnHQe4gXnWq/GHOoKSk9J2jM3MphPhNxYH1AwdzlXv un1cnWBZXPdqN2b9s5fv5M93WpKwuHNcdiF8KGIWqGCi0PgkLPoxlgiIwfC7Tsu98Am7 0SCh3zBoLxCAcLxoKou7v8V6DpPK7u0/uBuYnb7SE/pyQODlNeO+VjOTjLSzsNzDmIlr O15WnUA0bnI41QE8foVJ8cHpo3L7V0+V91SJCokCFsXzLqKpqvLQdRVoVdjFbJnDmEjM Zd8bY7Cx/XDQhUDGb3PjgP5Bv2f6BsJzu5wv9XsnrMLEOzBhmQu9DFWm80ksFB1/YHAY 9AzQ== X-Gm-Message-State: ABUngvfKXXMC0UVAoKNurC5Alz+YVLOXVVXdPliquc0DmoHxfmhAPnCD1Mdv4J54Jn9FKQ== X-Received: by 10.28.165.137 with SMTP id o131mr291601wme.50.1477683775730; Fri, 28 Oct 2016 12:42:55 -0700 (PDT) Received: from [192.168.0.23] (arf62-1-82-237-250-248.fbx.proxad.net. [82.237.250.248]) by smtp.googlemail.com with ESMTPSA id e5sm10475923wma.10.2016.10.28.12.42.54 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 28 Oct 2016 12:42:54 -0700 (PDT) To: "libstdc++@gcc.gnu.org" , gcc-patches From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= Subject: Default associative containers constructors/destructor/assignment Message-ID: <87049605-cb4e-a451-01f7-4cf106dd0633@gmail.com> Date: Fri, 28 Oct 2016 19:43:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------623EB949A9005422BC106CD5" X-SW-Source: 2016-10/txt/msg02384.txt.bz2 This is a multi-part message in MIME format. --------------623EB949A9005422BC106CD5 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-length: 2265 Hi Here is the patch to default all other associative containers operations that can be defaulted. To do so I introduce a _Rb_tree_key_compare type that take care of value initialization of compare functor. It also make sure that functor is copied rather than move in move constructor with necessary noexcept qualification. I also introduce _Rb_tree_header to take care of the initialization of the _Rb_tree_node_base used in the container header and of _M_node_count. I also use it to implement the move semantic and so default also _Rb_tree_impl move construtor. I also propose a solution for the FIXME regarding documentation of container destructor, I used C++11 default declaration. I don't have necessary tools to generate Doxygen doc but I am confident that it should work fine. I had to simplify doc for operations that are now defaulted. * include/bits/stl_map.h (map(const map&)): Make default. (map(map&&)): Likewise. (~map()): Likewise. (operator=(const map&)): Likewise. * include/bits/stl_multimap.h (multimap(const multimap&)): Make default. (multimap(multimap&&)): Likewise. (~multimap()): Likewise. (operator=(const multimap&)): Likewise. * include/bits/stl_set.h (set(const set&)): Make default. (set(set&&)): Likewise. (~set()): Likewise. (operator=(const set&)): Likewise. * include/bits/stl_multiset.h (multiset(const multiset&)): Make default. (multiset(multiset&&)): Likewise. (~multiset()): Likewise. (operator=(const multiset&)): Likewise. * include/bits/stl_tree.h (_Rb_tree_key_compare<>): New. (_Rb_tree_header): New. (_Rb_tree_impl): Inherit from latter. (_Rb_tree_impl()): Make default. (_Rb_tree_impl(const _Rb_tree_impl&)): New. (_Rb_tree_impl(_Rb_tree_impl&&)): New, default. (_Rb_tree_impl::_M_reset): Move... (_Rb_tree_header::_M_reset): ...here. (_Rb_tree_impl::_M_initialize): Move... (_Rb_tree_header::_M_initialize): ...here. (_Rb_tree(_Rb_tree&&)): Make default. (_Rb_tree_header::_M_move_data(_Rb_tree_header&)): New. (_Rb_tree<>::_M_move_data(_Rb_tree&, true_type)): Use latter. Tested under Linux x86_64, ok to commit ? François --------------623EB949A9005422BC106CD5 Content-Type: text/x-patch; name="default_tree.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="default_tree.patch" Content-length: 16559 diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h index dea7d5b..bbd0a97 100644 --- a/libstdc++-v3/include/bits/stl_map.h +++ b/libstdc++-v3/include/bits/stl_map.h @@ -185,25 +185,22 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER /** * @brief %Map copy constructor. - * @param __x A %map of identical element and allocator types. * - * The newly-created %map uses a copy of the allocator object used - * by @a __x (unless the allocator traits dictate a different object). + * Whether the allocator is copied depends on the allocator traits. */ +#if __cplusplus < 201103L map(const map& __x) : _M_t(__x._M_t) { } +#else + map(const map&) = default; -#if __cplusplus >= 201103L /** * @brief %Map move constructor. - * @param __x A %map of identical element and allocator types. * - * The newly-created %map contains the exact contents of @a __x. - * The contents of @a __x are a valid, but unspecified %map. + * The newly-created %map contains the exact contents of the moved + * instance. The moved instance is a valid, but unspecified, %map. */ - map(map&& __x) - noexcept(is_nothrow_copy_constructible<_Compare>::value) - : _M_t(std::move(__x._M_t)) { } + map(map&&) = default; /** * @brief Builds a %map from an initializer_list. @@ -284,31 +281,31 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER : _M_t(__comp, _Pair_alloc_type(__a)) { _M_t._M_insert_unique(__first, __last); } - // FIXME There is no dtor declared, but we should have something - // generated by Doxygen. I don't know what tags to add to this - // paragraph to make that happen: +#if __cplusplus >= 201103L /** * The dtor only erases the elements, and note that if the elements * themselves are pointers, the pointed-to memory is not touched in any * way. Managing the pointer is the user's responsibility. */ + ~map() = default; +#endif /** * @brief %Map assignment operator. - * @param __x A %map of identical element and allocator types. - * - * All the elements of @a __x are copied. * * Whether the allocator is copied depends on the allocator traits. */ +#if __cplusplus < 201103L map& operator=(const map& __x) { _M_t = __x._M_t; return *this; } +#else + map& + operator=(const map&) = default; -#if __cplusplus >= 201103L /// Move assignment operator. map& operator=(map&&) = default; diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h index 7e86b76..a5f775b 100644 --- a/libstdc++-v3/include/bits/stl_multimap.h +++ b/libstdc++-v3/include/bits/stl_multimap.h @@ -182,25 +182,23 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER /** * @brief %Multimap copy constructor. - * @param __x A %multimap of identical element and allocator types. * - * The newly-created %multimap uses a copy of the allocator object used - * by @a __x (unless the allocator traits dictate a different object). + * Whether the allocator is copied depends on the allocator traits. */ +#if __cplusplus < 201103L multimap(const multimap& __x) : _M_t(__x._M_t) { } +#else + multimap(const multimap&) = default; -#if __cplusplus >= 201103L /** * @brief %Multimap move constructor. - * @param __x A %multimap of identical element and allocator types. * - * The newly-created %multimap contains the exact contents of @a __x. - * The contents of @a __x are a valid, but unspecified %multimap. + * The newly-created %multimap contains the exact contents of the + * moved instance. The moved instance is a valid, but unspecified + * %multimap. */ - multimap(multimap&& __x) - noexcept(is_nothrow_copy_constructible<_Compare>::value) - : _M_t(std::move(__x._M_t)) { } + multimap(multimap&&) = default; /** * @brief Builds a %multimap from an initializer_list. @@ -278,31 +276,31 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER : _M_t(__comp, _Pair_alloc_type(__a)) { _M_t._M_insert_equal(__first, __last); } - // FIXME There is no dtor declared, but we should have something generated - // by Doxygen. I don't know what tags to add to this paragraph to make - // that happen: +#if __cplusplus >= 201103L /** * The dtor only erases the elements, and note that if the elements * themselves are pointers, the pointed-to memory is not touched in any * way. Managing the pointer is the user's responsibility. */ + ~multimap() = default; +#endif /** * @brief %Multimap assignment operator. - * @param __x A %multimap of identical element and allocator types. - * - * All the elements of @a __x are copied. * * Whether the allocator is copied depends on the allocator traits. */ +#if __cplusplus < 201103L multimap& operator=(const multimap& __x) { _M_t = __x._M_t; return *this; } +#else + multimap& + operator=(const multimap&) = default; -#if __cplusplus >= 201103L /// Move assignment operator. multimap& operator=(multimap&&) = default; diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h index 7fe2fbd..8a83b17 100644 --- a/libstdc++-v3/include/bits/stl_multiset.h +++ b/libstdc++-v3/include/bits/stl_multiset.h @@ -194,25 +194,23 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER /** * @brief %Multiset copy constructor. - * @param __x A %multiset of identical element and allocator types. * - * The newly-created %multiset uses a copy of the allocator object used - * by @a __x (unless the allocator traits dictate a different object). + * Whether the allocator is copied depends on the allocator traits. */ +#if __cplusplus < 201103L multiset(const multiset& __x) : _M_t(__x._M_t) { } +#else + multiset(const multiset&) = default; -#if __cplusplus >= 201103L /** * @brief %Multiset move constructor. - * @param __x A %multiset of identical element and allocator types. * - * The newly-created %multiset contains the exact contents of @a __x. - * The contents of @a __x are a valid, but unspecified %multiset. + * The newly-created %multiset contains the exact contents of the + * moved instance. The moved instance is a valid, but unspecified + * %multiset. */ - multiset(multiset&& __x) - noexcept(is_nothrow_copy_constructible<_Compare>::value) - : _M_t(std::move(__x._M_t)) { } + multiset(multiset&&) = default; /** * @brief Builds a %multiset from an initializer_list. @@ -256,24 +254,31 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER const allocator_type& __a) : _M_t(_Compare(), _Key_alloc_type(__a)) { _M_t._M_insert_equal(__first, __last); } + + /** + * The dtor only erases the elements, and note that if the elements + * themselves are pointers, the pointed-to memory is not touched in any + * way. Managing the pointer is the user's responsibility. + */ + ~multiset() = default; #endif /** * @brief %Multiset assignment operator. - * @param __x A %multiset of identical element and allocator types. - * - * All the elements of @a __x are copied. * * Whether the allocator is copied depends on the allocator traits. */ +#if __cplusplus < 201103L multiset& operator=(const multiset& __x) { _M_t = __x._M_t; return *this; } +#else + multiset& + operator=(const multiset&) = default; -#if __cplusplus >= 201103L /// Move assignment operator. multiset& operator=(multiset&&) = default; diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h index 5ed9672..db1e031 100644 --- a/libstdc++-v3/include/bits/stl_set.h +++ b/libstdc++-v3/include/bits/stl_set.h @@ -199,25 +199,22 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER /** * @brief %Set copy constructor. - * @param __x A %set of identical element and allocator types. * - * The newly-created %set uses a copy of the allocator object used - * by @a __x (unless the allocator traits dictate a different object). + * Whether the allocator is copied depends on the allocator traits. */ +#if __cplusplus < 201103L set(const set& __x) : _M_t(__x._M_t) { } +#else + set(const set&) = default; -#if __cplusplus >= 201103L /** * @brief %Set move constructor - * @param __x A %set of identical element and allocator types. * - * The newly-created %set contains the exact contents of @a x. - * The contents of @a x are a valid, but unspecified %set. + * The newly-created %set contains the exact contents of the moved + * instance. The moved instance is a valid, but unspecified, %set. */ - set(set&& __x) - noexcept(is_nothrow_copy_constructible<_Compare>::value) - : _M_t(std::move(__x._M_t)) { } + set(set&&) = default; /** * @brief Builds a %set from an initializer_list. @@ -261,24 +258,31 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER const allocator_type& __a) : _M_t(_Compare(), _Key_alloc_type(__a)) { _M_t._M_insert_unique(__first, __last); } + + /** + * The dtor only erases the elements, and note that if the elements + * themselves are pointers, the pointed-to memory is not touched in any + * way. Managing the pointer is the user's responsibility. + */ + ~set() = default; #endif /** * @brief %Set assignment operator. - * @param __x A %set of identical element and allocator types. - * - * All the elements of @a __x are copied. * * Whether the allocator is copied depends on the allocator traits. */ +#if __cplusplus < 201103L set& operator=(const set& __x) { _M_t = __x._M_t; return *this; } +#else + set& + operator=(const set&) = default; -#if __cplusplus >= 201103L /// Move assignment operator. set& operator=(set&&) = default; diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h index 0bc5f4b..126087e 100644 --- a/libstdc++-v3/include/bits/stl_tree.h +++ b/libstdc++-v3/include/bits/stl_tree.h @@ -137,6 +137,81 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } }; + // Helper type offering value initialization guaranty on the compare functor. + template + struct _Rb_tree_key_compare + { + _Key_compare _M_key_compare; + + _Rb_tree_key_compare() + _GLIBCXX_NOEXCEPT_IF( + is_nothrow_default_constructible<_Key_compare>::value) + : _M_key_compare() + { } + + _Rb_tree_key_compare(const _Key_compare& __comp) + : _M_key_compare(__comp) + { } + +#if __cplusplus >= 201103L + _Rb_tree_key_compare(_Rb_tree_key_compare&& __x) + noexcept(is_nothrow_copy_constructible<_Key_compare>::value) + : _M_key_compare(__x._M_key_compare) + { } +#endif + }; + + // Helper type to manage default initialization of node count and header. + struct _Rb_tree_header + { + _Rb_tree_node_base _M_header; + size_t _M_node_count; // Keeps track of size of tree. + + _Rb_tree_header() _GLIBCXX_NOEXCEPT + : _M_node_count(0) + { _M_initialize(); } + +#if __cplusplus >= 201103L + _Rb_tree_header(_Rb_tree_header&& __x) noexcept + : _M_node_count(0) + { + if (__x._M_header._M_parent != nullptr) + _M_move_data(__x); + else + _M_initialize(); + } + + void + _M_move_data(_Rb_tree_header& __x) + { + _M_header._M_parent = __x._M_header._M_parent; + _M_header._M_left = __x._M_header._M_left; + _M_header._M_right = __x._M_header._M_right; + _M_header._M_parent->_M_parent = &_M_header; + _M_node_count = __x._M_node_count; + + __x._M_reset(); + } +#endif + + void + _M_reset() + { + _M_initialize(); + _M_node_count = 0; + } + + private: + void + _M_initialize() + { + _M_header._M_color = _S_red; + _M_header._M_parent = 0; + _M_header._M_left = &_M_header; + _M_header._M_right = &_M_header; + } + }; + template struct _Rb_tree_node : public _Rb_tree_node_base { @@ -599,50 +674,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Unused _Is_pod_comparator is kept as it is part of mangled name. template - struct _Rb_tree_impl : public _Node_allocator + struct _Rb_tree_impl + : public _Node_allocator + , public _Rb_tree_key_compare<_Key_compare> + , public _Rb_tree_header { - _Key_compare _M_key_compare; - _Rb_tree_node_base _M_header; - size_type _M_node_count; // Keeps track of size of tree. + typedef _Rb_tree_key_compare<_Key_compare> _Base_key_compare; +#if __cplusplus < 201103L _Rb_tree_impl() - _GLIBCXX_NOEXCEPT_IF( - is_nothrow_default_constructible<_Node_allocator>::value - && is_nothrow_default_constructible<_Key_compare>::value) - : _Node_allocator(), _M_key_compare(), _M_header(), - _M_node_count(0) - { _M_initialize(); } + { } +#else + _Rb_tree_impl() = default; +#endif - _Rb_tree_impl(const _Key_compare& __comp, const _Node_allocator& __a) - : _Node_allocator(__a), _M_key_compare(__comp), _M_header(), - _M_node_count(0) - { _M_initialize(); } + _Rb_tree_impl(const _Rb_tree_impl& __x) + : _Node_allocator(_Alloc_traits::_S_select_on_copy(__x)) + , _Base_key_compare(__x._M_key_compare) + { } #if __cplusplus >= 201103L + _Rb_tree_impl(_Rb_tree_impl&&) = default; _Rb_tree_impl(const _Key_compare& __comp, _Node_allocator&& __a) - : _Node_allocator(std::move(__a)), _M_key_compare(__comp), - _M_header(), _M_node_count(0) - { _M_initialize(); } + : _Node_allocator(std::move(__a)), _Base_key_compare(__comp) + { } #endif - - void - _M_reset() - { - this->_M_header._M_parent = 0; - this->_M_header._M_left = &this->_M_header; - this->_M_header._M_right = &this->_M_header; - this->_M_node_count = 0; - } - - private: - void - _M_initialize() - { - this->_M_header._M_color = _S_red; - this->_M_header._M_parent = 0; - this->_M_header._M_left = &this->_M_header; - this->_M_header._M_right = &this->_M_header; - } }; _Rb_tree_impl<_Compare> _M_impl; @@ -845,8 +901,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _M_impl(__comp, _Node_allocator(__a)) { } _Rb_tree(const _Rb_tree& __x) - : _M_impl(__x._M_impl._M_key_compare, - _Alloc_traits::_S_select_on_copy(__x._M_get_Node_allocator())) + : _M_impl(__x._M_impl) { if (__x._M_root() != 0) { @@ -874,13 +929,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } - _Rb_tree(_Rb_tree&& __x) - : _M_impl(__x._M_impl._M_key_compare, - std::move(__x._M_get_Node_allocator())) - { - if (__x._M_root() != 0) - _M_move_data(__x, std::true_type()); - } + _Rb_tree(_Rb_tree&&) = default; _Rb_tree(_Rb_tree&& __x, const allocator_type& __a) : _Rb_tree(std::move(__x), _Node_allocator(__a)) @@ -1534,19 +1583,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void _Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>:: _M_move_data(_Rb_tree& __x, std::true_type) - { - _M_root() = __x._M_root(); - _M_leftmost() = __x._M_leftmost(); - _M_rightmost() = __x._M_rightmost(); - _M_root()->_M_parent = _M_end(); - - __x._M_root() = 0; - __x._M_leftmost() = __x._M_end(); - __x._M_rightmost() = __x._M_end(); - - this->_M_impl._M_node_count = __x._M_impl._M_node_count; - __x._M_impl._M_node_count = 0; - } + { _M_impl._M_move_data(__x._M_impl); } template --------------623EB949A9005422BC106CD5--