Simplify _Rb_tree instantiation

Message ID 302049ec-76ee-94c4-f0b1-11b508285144@gmail.com
State New
Headers show
Series
  • Simplify _Rb_tree instantiation
Related show

Commit Message

François Dumont May 25, 2018, 4:50 p.m.
Hi

As we are at working on associative containers I'd like to propose this 
last patch to remove the copy constructible constraint on the _Compare 
functor when it is supposed to be default constructed.

This way the _Compare is built directly at its final place.

     * include/bits/stl_tree.h (_Rb_tree_impl(_Node_allocator&&)): New.
     (_Rb_tree(const allocator_type&)): Use latter.
     * include/bits/stl_map.h (map(const allocator_type&)): Likewise.
     (map(initializer_list<value_type>, const allocator_type&)): Likewise.
     (map(_InputIterator, _InputIterator, const allocator_type&)): Likewise.
     * include/bits/stl_multimap.h
     (multimap(const allocator_type&)): Likewise.
     (multimap(initializer_list<value_type>, const allocator_type&)):
     Likewise.
     (multimap(_InputIterator, _InputIterator, const allocator_type&)):
     Likewise.
     * include/bits/stl_set.h (set(const allocator_type&)): Likewise.
     (set(initializer_list<value_type>, const allocator_type&)): Likewise.
     (set(_InputIterator, _InputIterator, const allocator_type&)): Likewise.
     * include/bits/stl_multiset.h
     (multiset(const allocator_type&)): Likewise.
     (multiset(initializer_list<value_type>, const allocator_type&)):
     Likewise.
     (multiset(_InputIterator, _InputIterator, const allocator_type&)):
     Likewise.

Tested under linux x86_64.

Ok to commit ?

François

Comments

Ville Voutilainen May 25, 2018, 6:35 p.m. | #1
On 25 May 2018 at 19:50, François Dumont <frs.dumont@gmail.com> wrote:
> Hi

>

> As we are at working on associative containers I'd like to propose this last

> patch to remove the copy constructible constraint on the _Compare functor

> when it is supposed to be default constructed.

>

> This way the _Compare is built directly at its final place.


Why is this patch removing _Compare() calls? That changes the initialization
of _Compare from value-initialization to default-initialization, which
is a breaking change.
Jonathan Wakely May 25, 2018, 7:16 p.m. | #2
On 25/05/18 21:35 +0300, Ville Voutilainen wrote:
>On 25 May 2018 at 19:50, François Dumont <frs.dumont@gmail.com> wrote:

>> Hi

>>

>> As we are at working on associative containers I'd like to propose this last

>> patch to remove the copy constructible constraint on the _Compare functor

>> when it is supposed to be default constructed.


The type is required to be CopyConstructible, so copying it into place
is confirming. But avoiding that copy might be more efficient, so
seems worthwhile.

>> This way the _Compare is built directly at its final place.

>

>Why is this patch removing _Compare() calls? That changes the initialization

>of _Compare from value-initialization to default-initialization, which

>is a breaking change.


The _Rb_tree_key_compare base class will still value-initialize it:

      _Rb_tree_key_compare()
      _GLIBCXX_NOEXCEPT_IF(
	is_nothrow_default_constructible<_Key_compare>::value)
      : _M_key_compare()
      { }
Ville Voutilainen May 25, 2018, 7:19 p.m. | #3
On 25 May 2018 at 22:16, Jonathan Wakely <jwakely@redhat.com> wrote:
>> Why is this patch removing _Compare() calls? That changes the

>> initialization

>> of _Compare from value-initialization to default-initialization, which

>> is a breaking change.

>

>

> The _Rb_tree_key_compare base class will still value-initialize it:

>

>      _Rb_tree_key_compare()

>      _GLIBCXX_NOEXCEPT_IF(

>         is_nothrow_default_constructible<_Key_compare>::value)

>      : _M_key_compare()

>      { }


Okay, no problem then.
François Dumont May 27, 2018, 5:10 p.m. | #4
On 25/05/2018 21:19, Ville Voutilainen wrote:
> On 25 May 2018 at 22:16, Jonathan Wakely <jwakely@redhat.com> wrote:

>>> Why is this patch removing _Compare() calls? That changes the

>>> initialization

>>> of _Compare from value-initialization to default-initialization, which

>>> is a breaking change.

>>

>> The _Rb_tree_key_compare base class will still value-initialize it:

>>

>>       _Rb_tree_key_compare()

>>       _GLIBCXX_NOEXCEPT_IF(

>>          is_nothrow_default_constructible<_Key_compare>::value)

>>       : _M_key_compare()

>>       { }

> Okay, no problem then.

>

Now applied, note that I eventually restored the explicit calls to 
_M_t() in the template iterator range constructors. We didn't talk about 
it and there is also such a call for the default constructors in 
pre-c++11 mode so I prefered to keep consistency.

François

Patch

diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h
index 773b72e..25ab25d 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -232,7 +232,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /// Allocator-extended default constructor.
       explicit
       map(const allocator_type& __a)
-      : _M_t(_Compare(), _Pair_alloc_type(__a)) { }
+      : _M_t(_Pair_alloc_type(__a)) { }
 
       /// Allocator-extended copy constructor.
       map(const map& __m, const allocator_type& __a)
@@ -246,14 +246,14 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       /// Allocator-extended initialier-list constructor.
       map(initializer_list<value_type> __l, const allocator_type& __a)
-      : _M_t(_Compare(), _Pair_alloc_type(__a))
+      : _M_t(_Pair_alloc_type(__a))
       { _M_t._M_insert_unique(__l.begin(), __l.end()); }
 
       /// Allocator-extended range constructor.
       template<typename _InputIterator>
 	map(_InputIterator __first, _InputIterator __last,
 	    const allocator_type& __a)
-	: _M_t(_Compare(), _Pair_alloc_type(__a))
+	: _M_t(_Pair_alloc_type(__a))
 	{ _M_t._M_insert_unique(__first, __last); }
 #endif
 
@@ -269,7 +269,6 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        */
       template<typename _InputIterator>
 	map(_InputIterator __first, _InputIterator __last)
-	: _M_t()
 	{ _M_t._M_insert_unique(__first, __last); }
 
       /**
diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h
index 9845c01..fcfea88 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -229,7 +229,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /// Allocator-extended default constructor.
       explicit
       multimap(const allocator_type& __a)
-      : _M_t(_Compare(), _Pair_alloc_type(__a)) { }
+      : _M_t(_Pair_alloc_type(__a)) { }
 
       /// Allocator-extended copy constructor.
       multimap(const multimap& __m, const allocator_type& __a)
@@ -243,14 +243,14 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       /// Allocator-extended initialier-list constructor.
       multimap(initializer_list<value_type> __l, const allocator_type& __a)
-      : _M_t(_Compare(), _Pair_alloc_type(__a))
+      : _M_t(_Pair_alloc_type(__a))
       { _M_t._M_insert_equal(__l.begin(), __l.end()); }
 
       /// Allocator-extended range constructor.
       template<typename _InputIterator>
 	multimap(_InputIterator __first, _InputIterator __last,
 		 const allocator_type& __a)
-	: _M_t(_Compare(), _Pair_alloc_type(__a))
+	: _M_t(_Pair_alloc_type(__a))
 	{ _M_t._M_insert_equal(__first, __last); }
 #endif
 
@@ -265,7 +265,6 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        */
       template<typename _InputIterator>
 	multimap(_InputIterator __first, _InputIterator __last)
-	: _M_t()
 	{ _M_t._M_insert_equal(__first, __last); }
 
       /**
diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h
index 38159ab..1155e55 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -185,7 +185,6 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        */
       template<typename _InputIterator>
 	multiset(_InputIterator __first, _InputIterator __last)
-	: _M_t()
 	{ _M_t._M_insert_equal(__first, __last); }
 
       /**
@@ -245,7 +244,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /// Allocator-extended default constructor.
       explicit
       multiset(const allocator_type& __a)
-      : _M_t(_Compare(), _Key_alloc_type(__a)) { }
+      : _M_t(_Key_alloc_type(__a)) { }
 
       /// Allocator-extended copy constructor.
       multiset(const multiset& __m, const allocator_type& __a)
@@ -259,14 +258,14 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       /// Allocator-extended initialier-list constructor.
       multiset(initializer_list<value_type> __l, const allocator_type& __a)
-      : _M_t(_Compare(), _Key_alloc_type(__a))
+      : _M_t(_Key_alloc_type(__a))
       { _M_t._M_insert_equal(__l.begin(), __l.end()); }
 
       /// Allocator-extended range constructor.
       template<typename _InputIterator>
 	multiset(_InputIterator __first, _InputIterator __last,
 		 const allocator_type& __a)
-	: _M_t(_Compare(), _Key_alloc_type(__a))
+	: _M_t(_Key_alloc_type(__a))
 	{ _M_t._M_insert_equal(__first, __last); }
 
       /**
diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h
index 913af9d..1dcec9f 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -189,7 +189,6 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        */
       template<typename _InputIterator>
 	set(_InputIterator __first, _InputIterator __last)
-	: _M_t()
 	{ _M_t._M_insert_unique(__first, __last); }
 
       /**
@@ -249,7 +248,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /// Allocator-extended default constructor.
       explicit
       set(const allocator_type& __a)
-      : _M_t(_Compare(), _Key_alloc_type(__a)) { }
+      : _M_t(_Key_alloc_type(__a)) { }
 
       /// Allocator-extended copy constructor.
       set(const set& __x, const allocator_type& __a)
@@ -263,14 +262,14 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       /// Allocator-extended initialier-list constructor.
       set(initializer_list<value_type> __l, const allocator_type& __a)
-      : _M_t(_Compare(), _Key_alloc_type(__a))
+      : _M_t(_Key_alloc_type(__a))
       { _M_t._M_insert_unique(__l.begin(), __l.end()); }
 
       /// Allocator-extended range constructor.
       template<typename _InputIterator>
 	set(_InputIterator __first, _InputIterator __last,
 	    const allocator_type& __a)
-	: _M_t(_Compare(), _Key_alloc_type(__a))
+	: _M_t(_Key_alloc_type(__a))
 	{ _M_t._M_insert_unique(__first, __last); }
 
       /**
diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index c4f08d2..a4bbee4 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -753,6 +753,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #else
 	  _Rb_tree_impl(_Rb_tree_impl&&) = default;
 
+	  explicit
+	  _Rb_tree_impl(_Node_allocator&& __a)
+	  : _Node_allocator(std::move(__a))
+	  { }
+
 	  _Rb_tree_impl(_Rb_tree_impl&& __x, _Node_allocator&& __a)
 	  : _Node_allocator(std::move(__a)),
 	    _Base_key_compare(std::move(__x)),
@@ -986,7 +991,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 #if __cplusplus >= 201103L
       _Rb_tree(const allocator_type& __a)
-      : _M_impl(_Compare(), _Node_allocator(__a))
+      : _M_impl(_Node_allocator(__a))
       { }
 
       _Rb_tree(const _Rb_tree& __x, const allocator_type& __a)