[PATCH] 77864 Fix noexcept conditions for map/set default constructors

[PATCH] 77864 Fix noexcept conditions for map/set default constructors

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 841 bytes --]

I added conditional noexcept to maps and sets, but forgot to account
for the comparison function, which could throw when constructed.

	PR libstdc++/77864
	* include/bits/stl_map.h (map::map()): Use nothrow constructibility
	of comparison function in conditional noexcept.
	* include/bits/stl_multimap.h (multimap::multimap()): Likewise.
	* include/bits/stl_multiset.h (multiset::multiset()): Likewise.
	* include/bits/stl_set.h (set::set()): Likewise.
	* testsuite/23_containers/map/cons/noexcept_default_construct.cc:
	New test.
	* testsuite/23_containers/multimap/cons/noexcept_default_construct.cc:
	Likewise.
	* testsuite/23_containers/multiset/cons/noexcept_default_construct.cc:
	Likewise.
	* testsuite/23_containers/set/cons/noexcept_default_construct.cc:
	Likewise.

Tested powerpc64le-linux, committing to trunk, gcc-5 and gcc-6.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 9381 bytes --]

commit 8cf0acf15ae71abeb29c0cf5b900131a20b6f375
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Oct 5 12:20:32 2016 +0100

    77864 Fix noexcept conditions for map/set default constructors
    
    	PR libstdc++/77864
    	* include/bits/stl_map.h (map::map()): Use nothrow constructibility
    	of comparison function in conditional noexcept.
    	* include/bits/stl_multimap.h (multimap::multimap()): Likewise.
    	* include/bits/stl_multiset.h (multiset::multiset()): Likewise.
    	* include/bits/stl_set.h (set::set()): Likewise.
    	* testsuite/23_containers/map/cons/noexcept_default_construct.cc:
    	New test.
    	* testsuite/23_containers/multimap/cons/noexcept_default_construct.cc:
    	Likewise.
    	* testsuite/23_containers/multiset/cons/noexcept_default_construct.cc:
    	Likewise.
    	* testsuite/23_containers/set/cons/noexcept_default_construct.cc:
    	Likewise.

diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h
index 9a0454a..e5b2a1b 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -168,9 +168,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        *  @brief  Default constructor creates no elements.
        */
       map()
-#if __cplusplus >= 201103L
-      noexcept(is_nothrow_default_constructible<allocator_type>::value)
-#endif
+      _GLIBCXX_NOEXCEPT_IF(
+	  is_nothrow_default_constructible<allocator_type>::value
+	  && is_nothrow_default_constructible<key_compare>::value)
       : _M_t() { }
 
       /**
diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h
index c794b9b..d240427 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -165,9 +165,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        *  @brief  Default constructor creates no elements.
        */
       multimap()
-#if __cplusplus >= 201103L
-      noexcept(is_nothrow_default_constructible<allocator_type>::value)
-#endif
+      _GLIBCXX_NOEXCEPT_IF(
+	  is_nothrow_default_constructible<allocator_type>::value
+	  && is_nothrow_default_constructible<key_compare>::value)
       : _M_t() { }
 
       /**
diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h
index d3219eb..cc068a9 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -145,9 +145,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        *  @brief  Default constructor creates no elements.
        */
       multiset()
-#if __cplusplus >= 201103L
-      noexcept(is_nothrow_default_constructible<allocator_type>::value)
-#endif
+      _GLIBCXX_NOEXCEPT_IF(
+	  is_nothrow_default_constructible<allocator_type>::value
+	  && is_nothrow_default_constructible<key_compare>::value)
       : _M_t() { }
 
       /**
diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h
index 140db39..3938351 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -148,9 +148,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        *  @brief  Default constructor creates no elements.
        */
       set()
-#if __cplusplus >= 201103L
-      noexcept(is_nothrow_default_constructible<allocator_type>::value)
-#endif
+      _GLIBCXX_NOEXCEPT_IF(
+	  is_nothrow_default_constructible<allocator_type>::value
+	  && is_nothrow_default_constructible<key_compare>::value)
       : _M_t() { }
 
       /**
diff --git a/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_default_construct.cc b/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_default_construct.cc
new file mode 100644
index 0000000..1286d37
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_default_construct.cc
@@ -0,0 +1,32 @@
+// Copyright (C) 2016 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
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do compile { target c++11 } }
+
+#include <map>
+
+using mtype1 = std::map<int, int>;
+static_assert(std::is_nothrow_default_constructible<mtype1>::value, "Error");
+
+struct cmp
+{
+  cmp() { }
+  bool operator()(int, int) const;
+};
+
+using mtype2 = std::map<int, int, cmp>;
+static_assert( !std::is_nothrow_default_constructible<mtype2>::value, "Error");
diff --git a/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_default_construct.cc b/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_default_construct.cc
new file mode 100644
index 0000000..30c0656
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_default_construct.cc
@@ -0,0 +1,32 @@
+// Copyright (C) 2016 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
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do compile { target c++11 } }
+
+#include <map>
+
+using mtype1 = std::multimap<int, int>;
+static_assert(std::is_nothrow_default_constructible<mtype1>::value, "Error");
+
+struct cmp
+{
+  cmp() { }
+  bool operator()(int, int) const;
+};
+
+using mtype2 = std::multimap<int, int, cmp>;
+static_assert( !std::is_nothrow_default_constructible<mtype2>::value, "Error");
diff --git a/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_default_construct.cc b/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_default_construct.cc
new file mode 100644
index 0000000..d42fcd5
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_default_construct.cc
@@ -0,0 +1,32 @@
+// Copyright (C) 2016 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
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do compile { target c++11 } }
+
+#include <set>
+
+using stype1 = std::multiset<int>;
+static_assert(std::is_nothrow_default_constructible<stype1>::value, "Error");
+
+struct cmp
+{
+  cmp() { }
+  bool operator()(int, int) const;
+};
+
+using stype2 = std::multiset<int, cmp>;
+static_assert( !std::is_nothrow_default_constructible<stype2>::value, "Error");
diff --git a/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_default_construct.cc b/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_default_construct.cc
new file mode 100644
index 0000000..7aba006
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_default_construct.cc
@@ -0,0 +1,32 @@
+// Copyright (C) 2016 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
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do compile { target c++11 } }
+
+#include <set>
+
+using stype1 = std::set<int>;
+static_assert(std::is_nothrow_default_constructible<stype1>::value, "Error");
+
+struct cmp
+{
+  cmp() { }
+  bool operator()(int, int) const;
+};
+
+using stype2 = std::set<int, cmp>;
+static_assert( !std::is_nothrow_default_constructible<stype2>::value, "Error");

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[Marc Glisse]

On Wed, 5 Oct 2016, Jonathan Wakely wrote:

> I added conditional noexcept to maps and sets, but forgot to account
> for the comparison function, which could throw when constructed.

IMO you are fighting a losing battle. We should implement noexcept(auto) 
(possibly with some private __noexcept_auto__ spelling) and just use that 
in most places where we want a conditional noexcept.

-- 
Marc Glisse

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[Jonathan Wakely]

On 05/10/16 14:10 +0200, Marc Glisse wrote:
>On Wed, 5 Oct 2016, Jonathan Wakely wrote:
>
>>I added conditional noexcept to maps and sets, but forgot to account
>>for the comparison function, which could throw when constructed.
>
>IMO you are fighting a losing battle. We should implement 
>noexcept(auto) (possibly with some private __noexcept_auto__ spelling) 
>and just use that in most places where we want a conditional noexcept.

That would be nice, but beyond my ability :-)

So until then we can either remove the exception specs entirely, or
make them correct.

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[François Dumont]

[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]

On 05/10/2016 14:13, Jonathan Wakely wrote:
> On 05/10/16 14:10 +0200, Marc Glisse wrote:
>> On Wed, 5 Oct 2016, Jonathan Wakely wrote:
>>
>>> I added conditional noexcept to maps and sets, but forgot to account
>>> for the comparison function, which could throw when constructed.
>>
>> IMO you are fighting a losing battle. We should implement 
>> noexcept(auto) (possibly with some private __noexcept_auto__ 
>> spelling) and just use that in most places where we want a 
>> conditional noexcept.
>
> That would be nice, but beyond my ability :-)
>
> So until then we can either remove the exception specs entirely, or
> make them correct.
>
>
Another approach is to rely on existing compiler ability to compute 
conditional noexcept when defaulting implementations. This is what I 
have done in this patch.

The new default constructor on _Rb_tree_node_base is not a problem as it 
is not used to build _Rb_tree_node.

I'll try to do the same for copy constructor/assignment and move 
constructor/assignment.

     * include/bits/stl_map.h (map()): Make default.
     * include/bits/stl_multimap.h (multimap()): Likewise.
     * include/bits/stl_multiset.h (multiset()): Likewise.
     * include/bits/stl_set.h (set()): Likewise.
     * include/bits/stl_tree.h (_Rb_tree_node_base()): New.
     (_Rb_tree_impl()): Make default.
     (_Rb_tree_impl::_M_initialize()): Delete.
     (_Rb_tree()): Make default.

Tested under Linux x86_64, ok to commit ?

François


[-- Attachment #2: default_dflt_const.patch --]
[-- Type: text/x-patch, Size: 5204 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h
index e5b2a1b..dea7d5b 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -167,11 +167,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      map()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      map() : _M_t() { }
+#else
+      map() = default;
+#endif
 
       /**
        *  @brief  Creates a %map with no elements.
diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h
index d240427..7e86b76 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -164,11 +164,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      multimap()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      multimap() : _M_t() { }
+#else
+      multimap() = default;
+#endif
 
       /**
        *  @brief  Creates a %multimap with no elements.
diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h
index cc068a9..7fe2fbd 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -144,11 +144,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      multiset()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      multiset() : _M_t() { }
+#else
+      multiset() = default;
+#endif
 
       /**
        *  @brief  Creates a %multiset with no elements.
diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h
index 3938351..5ed9672 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -147,11 +147,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      set()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      set() : _M_t() { }
+#else
+      set() = default;
+#endif
 
       /**
        *  @brief  Creates a %set with no elements.
diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index ee2dc70..ed575d0 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -108,6 +108,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     _Base_ptr		_M_left;
     _Base_ptr		_M_right;
 
+    _Rb_tree_node_base() _GLIBCXX_NOEXCEPT
+      : _M_color(_S_red), _M_parent(0), _M_left(this), _M_right(this)
+    { }
+
     static _Base_ptr
     _S_minimum(_Base_ptr __x) _GLIBCXX_NOEXCEPT
     {
@@ -603,23 +607,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
         {
 	  _Key_compare		_M_key_compare;
 	  _Rb_tree_node_base 	_M_header;
+#if __cplusplus < 201103L
 	  size_type 		_M_node_count; // Keeps track of size of tree.
+#else
+	  size_type 		_M_node_count = 0; // Keeps track of size of tree.
+#endif
 
+#if __cplusplus < 201103L
 	  _Rb_tree_impl()
 	  : _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(); }
+	  : _Node_allocator(__a), _M_key_compare(__comp), _M_header()
+#if __cplusplus < 201103L
+	  , _M_node_count(0)
+#endif
+	  { }
 
 #if __cplusplus >= 201103L
 	  _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)), _M_key_compare(__comp),
+	      _M_header()
+	  { }
 #endif
 
 	  void
@@ -630,16 +644,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    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;
@@ -831,7 +835,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     public:
       // allocation/deallocation
+#if __cplusplus < 201103L
       _Rb_tree() { }
+#else
+      _Rb_tree() = default;
+#endif
 
       _Rb_tree(const _Compare& __comp,
 	       const allocator_type& __a = allocator_type())

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[Jonathan Wakely]

On 06/10/16 22:17 +0200, François Dumont wrote:
>Another approach is to rely on existing compiler ability to compute 
>conditional noexcept when defaulting implementations. This is what I 
>have done in this patch.
>
>The new default constructor on _Rb_tree_node_base is not a problem as 
>it is not used to build _Rb_tree_node.

Why not?

>I'll try to do the same for copy constructor/assignment and move 
>constructor/assignment.

We need to make sure we don't change whether any of those operations
are trivial (which shouldn't be a problem for copy/move, because they
are definitely very non-trivial and will stay that way!)

Does this change the default constructors from non-trivial to trivial?


>--- a/libstdc++-v3/include/bits/stl_tree.h
>+++ b/libstdc++-v3/include/bits/stl_tree.h
>@@ -108,6 +108,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     _Base_ptr		_M_left;
>     _Base_ptr		_M_right;
> 
>+    _Rb_tree_node_base() _GLIBCXX_NOEXCEPT
>+      : _M_color(_S_red), _M_parent(0), _M_left(this), _M_right(this)
>+    { }
>+
>     static _Base_ptr
>     _S_minimum(_Base_ptr __x) _GLIBCXX_NOEXCEPT
>     {

Another option would be:

    struct _Head_node : _Rb_tree_node_base {
      _Head_node() {
        _M_color = _S_red;
        _M_parent = _Base_ptr();
        _M_left = _M_right = this;
      }
    };

>@@ -603,23 +607,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>         {
> 	  _Key_compare		_M_key_compare;
> 	  _Rb_tree_node_base 	_M_header;

          _Head_node _M_header;

That way *only* this node gets the zero-initialization, not all node
bases.

With either solution we can get rid of _M_header() in every
ctor-initializer-list.


>+#if __cplusplus < 201103L
> 	  size_type 		_M_node_count; // Keeps track of size of tree.
>+#else
>+	  size_type 		_M_node_count = 0; // Keeps track of size of tree.
>+#endif
> 
>+#if __cplusplus < 201103L
> 	  _Rb_tree_impl()
> 	  : _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(); }
>+	  : _Node_allocator(__a), _M_key_compare(__comp), _M_header()
>+#if __cplusplus < 201103L
>+	  , _M_node_count(0)
>+#endif

Doing this conditionally seems pointless, why not just set it here
unconditionally?

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[François Dumont]

On 06/10/2016 23:34, Jonathan Wakely wrote:
> On 06/10/16 22:17 +0200, François Dumont wrote:
>> Another approach is to rely on existing compiler ability to compute 
>> conditional noexcept when defaulting implementations. This is what I 
>> have done in this patch.
>>
>> The new default constructor on _Rb_tree_node_base is not a problem as 
>> it is not used to build _Rb_tree_node.
>
> Why not?

_Rb_tree_node_base is used in 2 context. As member of _Rb_tree_impl in 
which case we need the new default constructor. And also as base class 
of _Rb_tree_node which is never constructed. Nodes are being allocated 
and then associated value is being constructed through the allocator, 
the node default constructor itself is never invoked.

     If you think it is cleaner to create an intermediate type that will 
take care of this initialization through its default constructor I can 
do that.

>
>> I'll try to do the same for copy constructor/assignment and move 
>> constructor/assignment.
>
> We need to make sure we don't change whether any of those operations
> are trivial (which shouldn't be a problem for copy/move, because they
> are definitely very non-trivial and will stay that way!)
>
> Does this change the default constructors from non-trivial to trivial?
It would be a major compiler bug if making a constructor default was 
making it trivial.
>
>
>> --- a/libstdc++-v3/include/bits/stl_tree.h
>> +++ b/libstdc++-v3/include/bits/stl_tree.h
>> @@ -108,6 +108,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>     _Base_ptr        _M_left;
>>     _Base_ptr        _M_right;
>>
>> +    _Rb_tree_node_base() _GLIBCXX_NOEXCEPT
>> +      : _M_color(_S_red), _M_parent(0), _M_left(this), _M_right(this)
>> +    { }
>> +
>>     static _Base_ptr
>>     _S_minimum(_Base_ptr __x) _GLIBCXX_NOEXCEPT
>>     {
>
> Another option would be:
>
>    struct _Head_node : _Rb_tree_node_base {
>      _Head_node() {
>        _M_color = _S_red;
>        _M_parent = _Base_ptr();
>        _M_left = _M_right = this;
>      }
>    };
If you want something like that I would rather do:

_Rb_tree_node_base() = default;
_Rb_tree_node_base(int) _GLIBCXX_NO_EXCEPT
       : _M_color(_S_red), _M_parent(0), _M_left(this), _M_right(this)
     { }

and then:

struct _Head_node : _Rb_tree_node_base {
     _Head_node() _GLIBCXX_NO_EXCEPT
     : _Rb_tree_node_base(0)
     { }
};

but as I already said the default constructor on _Rb_tree_node_base 
works just fine.

>
>> @@ -603,23 +607,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>         {
>>       _Key_compare        _M_key_compare;
>>       _Rb_tree_node_base     _M_header;
>
>          _Head_node _M_header;
>
> That way *only* this node gets the zero-initialization, not all node
> bases.
>
> With either solution we can get rid of _M_header() in every
> ctor-initializer-list.
I am already preparing another patch, I will add this simplification.
>
>
>> +#if __cplusplus < 201103L
>>       size_type         _M_node_count; // Keeps track of size of tree.
>> +#else
>> +      size_type         _M_node_count = 0; // Keeps track of size of 
>> tree.
>> +#endif
>>
>> +#if __cplusplus < 201103L
>>       _Rb_tree_impl()
>>       : _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(); }
>> +      : _Node_allocator(__a), _M_key_compare(__comp), _M_header()
>> +#if __cplusplus < 201103L
>> +      , _M_node_count(0)
>> +#endif
>
> Doing this conditionally seems pointless, why not just set it here
> unconditionally
I wasn't sure about this one, just seemed clearner to not do this 0 
initialization again as already done at class definition level. If 
compiler just moved it away then ok, I can remove the check.

François

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[Jonathan Wakely]

On 08/10/16 22:55 +0200, François Dumont wrote:
>On 06/10/2016 23:34, Jonathan Wakely wrote:
>>On 06/10/16 22:17 +0200, François Dumont wrote:
>>>Another approach is to rely on existing compiler ability to 
>>>compute conditional noexcept when defaulting implementations. This 
>>>is what I have done in this patch.
>>>
>>>The new default constructor on _Rb_tree_node_base is not a problem 
>>>as it is not used to build _Rb_tree_node.
>>
>>Why not?
>
>_Rb_tree_node_base is used in 2 context. As member of _Rb_tree_impl in 
>which case we need the new default constructor. And also as base class 
>of _Rb_tree_node which is never constructed. Nodes are being allocated 
>and then associated value is being constructed through the allocator, 
>the node default constructor itself is never invoked.

In C++03 mode that is true, but it's only valid because the type is
trivially-constructible. If the type requires "non-vacuous
initialization" then it's not valid to allocate memory for it and
start using it without invoking a constructor. If you add a
non-trivial constructor then we can't do that any more.

In C++11 and later, see line 550 in <bits/stl_tree.h>

        ::new(__node) _Rb_tree_node<_Val>;

This default-constructs a tree node. Currently there is no
user-provided default constructor, so default-construction does no
initialization. Adding your constructor would mean it is used for
every node.


>    If you think it is cleaner to create an intermediate type that 
>will take care of this initialization through its default constructor 
>I can do that.
>
>>
>>>I'll try to do the same for copy constructor/assignment and move 
>>>constructor/assignment.
>>
>>We need to make sure we don't change whether any of those operations
>>are trivial (which shouldn't be a problem for copy/move, because they
>>are definitely very non-trivial and will stay that way!)
>>
>>Does this change the default constructors from non-trivial to trivial?
>It would be a major compiler bug if making a constructor default was 
>making it trivial.

I must be misunderstanding you, because this is not a bug:

#include <type_traits>

struct A {
  A() { }
};

static_assert( !std::is_trivially_default_constructible<A>::value, "" );

struct B {
  B() = default;
};

static_assert( std::is_trivially_default_constructible<B>::value, "" );

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[Jonathan Wakely]

On 09/10/16 16:14 +0100, Jonathan Wakely wrote:
>On 08/10/16 22:55 +0200, François Dumont wrote:
>>On 06/10/2016 23:34, Jonathan Wakely wrote:
>>>On 06/10/16 22:17 +0200, François Dumont wrote:
>>>>Another approach is to rely on existing compiler ability to 
>>>>compute conditional noexcept when defaulting implementations. 
>>>>This is what I have done in this patch.
>>>>
>>>>The new default constructor on _Rb_tree_node_base is not a 
>>>>problem as it is not used to build _Rb_tree_node.
>>>
>>>Why not?
>>
>>_Rb_tree_node_base is used in 2 context. As member of _Rb_tree_impl 
>>in which case we need the new default constructor. And also as base 
>>class of _Rb_tree_node which is never constructed. Nodes are being 
>>allocated and then associated value is being constructed through the 
>>allocator, the node default constructor itself is never invoked.
>
>In C++03 mode that is true, but it's only valid because the type is
>trivially-constructible. If the type requires "non-vacuous
>initialization" then it's not valid to allocate memory for it and
>start using it without invoking a constructor. If you add a
>non-trivial constructor then we can't do that any more.

In fact that code is highly questionable, because the element member
makes the node require non-trivial initialization. We rely on the base
being trivially-constructible, and the element being constructed by
the allocator, and assume that it's OK to then treat that memory as a
node. In fact only its base class and member have been constructed,
not the node itself. The C++11 version (using an aligned buffer) is
correct. But I'd prefer not to make the C++98 version worse.

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[François Dumont]

[-- Attachment #1: Type: text/plain, Size: 2674 bytes --]

On 09/10/2016 17:14, Jonathan Wakely wrote:
> On 08/10/16 22:55 +0200, François Dumont wrote:
>> On 06/10/2016 23:34, Jonathan Wakely wrote:
>>> On 06/10/16 22:17 +0200, François Dumont wrote:
>>>> Another approach is to rely on existing compiler ability to compute 
>>>> conditional noexcept when defaulting implementations. This is what 
>>>> I have done in this patch.
>>>>
>>>> The new default constructor on _Rb_tree_node_base is not a problem 
>>>> as it is not used to build _Rb_tree_node.
>>>
>>> Why not?
>>
>> _Rb_tree_node_base is used in 2 context. As member of _Rb_tree_impl 
>> in which case we need the new default constructor. And also as base 
>> class of _Rb_tree_node which is never constructed. Nodes are being 
>> allocated and then associated value is being constructed through the 
>> allocator, the node default constructor itself is never invoked.
>
> In C++03 mode that is true, but it's only valid because the type is
> trivially-constructible. If the type requires "non-vacuous
> initialization" then it's not valid to allocate memory for it and
> start using it without invoking a constructor.

   Good to know.

> If you add a
> non-trivial constructor then we can't do that any more.
>
> In C++11 and later, see line 550 in <bits/stl_tree.h>
>
>        ::new(__node) _Rb_tree_node<_Val>;
>
> This default-constructs a tree node. Currently there is no
> user-provided default constructor, so default-construction does no
> initialization. Adding your constructor would mean it is used for
> every node.

I missed this call, indeed. I should have deleted default constructor 
and run compilation to be sure.

>
>>    If you think it is cleaner to create an intermediate type that 
>> will take care of this initialization through its default constructor 
>> I can do that.
>>
>>>
>>>> I'll try to do the same for copy constructor/assignment and move 
>>>> constructor/assignment.
>>>
>>> We need to make sure we don't change whether any of those operations
>>> are trivial (which shouldn't be a problem for copy/move, because they
>>> are definitely very non-trivial and will stay that way!)
>>>
>>> Does this change the default constructors from non-trivial to trivial?
>> It would be a major compiler bug if making a constructor default was 
>> making it trivial.
>
> I must be misunderstanding you, because this is not a bug:

No, my fault, I was misunderstanding you. Now that I know about validity 
of using a "non-constructed" type only if trivial, it is much clearer.

So here is the fixed patch with your proposed intermediate type 
containing the necessary default constructor.

Being tested, ok to commit if successful ?

François


[-- Attachment #2: default_dflt_const.patch --]
[-- Type: text/x-patch, Size: 5339 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h
index e5b2a1b..dea7d5b 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -167,11 +167,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      map()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      map() : _M_t() { }
+#else
+      map() = default;
+#endif
 
       /**
        *  @brief  Creates a %map with no elements.
diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h
index d240427..7e86b76 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -164,11 +164,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      multimap()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      multimap() : _M_t() { }
+#else
+      multimap() = default;
+#endif
 
       /**
        *  @brief  Creates a %multimap with no elements.
diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h
index cc068a9..7fe2fbd 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -144,11 +144,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      multiset()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      multiset() : _M_t() { }
+#else
+      multiset() = default;
+#endif
 
       /**
        *  @brief  Creates a %multiset with no elements.
diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h
index 3938351..5ed9672 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -147,11 +147,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      set()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      set() : _M_t() { }
+#else
+      set() = default;
+#endif
 
       /**
        *  @brief  Creates a %set with no elements.
diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index ee2dc70..f2928f2 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -137,6 +137,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
   };
 
+  struct _Rb_header_node : public _Rb_tree_node_base
+  {
+    _Rb_header_node() _GLIBCXX_NOEXCEPT
+    {
+      _M_color = _S_red;
+      _M_parent = _Base_ptr();
+      _M_left = _M_right = this;
+    }
+  };
+
   template<typename _Val>
     struct _Rb_tree_node : public _Rb_tree_node_base
     {
@@ -602,24 +612,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
         struct _Rb_tree_impl : public _Node_allocator
         {
 	  _Key_compare		_M_key_compare;
-	  _Rb_tree_node_base 	_M_header;
+	  _Rb_header_node 	_M_header;
+#if __cplusplus < 201103L
 	  size_type 		_M_node_count; // Keeps track of size of tree.
+#else
+	  size_type 		_M_node_count = 0; // Keeps track of size of tree.
+#endif
 
+#if __cplusplus < 201103L
 	  _Rb_tree_impl()
-	  : _Node_allocator(), _M_key_compare(), _M_header(),
-	    _M_node_count(0)
-	  { _M_initialize(); }
+	  : _M_node_count(0)
+	  { }
+#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(); }
+	  : _Node_allocator(__a), _M_key_compare(__comp)
+#if __cplusplus < 201103L
+	  , _M_node_count(0)
+#endif
+	  { }
 
 #if __cplusplus >= 201103L
 	  _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)), _M_key_compare(__comp)
+	  { }
 #endif
 
 	  void
@@ -630,16 +648,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    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;
@@ -831,7 +839,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     public:
       // allocation/deallocation
+#if __cplusplus < 201103L
       _Rb_tree() { }
+#else
+      _Rb_tree() = default;
+#endif
 
       _Rb_tree(const _Compare& __comp,
 	       const allocator_type& __a = allocator_type())

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[Tim Song]

Trying again...with a few edits.

> On Mon, Oct 10, 2016 at 3:24 PM, François Dumont <frs.dumont@gmail.com>
> wrote:
>
> @@ -602,24 +612,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>          struct _Rb_tree_impl : public _Node_allocator
>          {
>    _Key_compare _M_key_compare;
> -  _Rb_tree_node_base _M_header;
> +  _Rb_header_node _M_header;
> +#if __cplusplus < 201103L
>    size_type _M_node_count; // Keeps track of size of tree.
> +#else
> +  size_type _M_node_count = 0; // Keeps track of size of tree.
> +#endif
>
> +#if __cplusplus < 201103L
>    _Rb_tree_impl()
> -  : _Node_allocator(), _M_key_compare(), _M_header(),
> -    _M_node_count(0)
> -  { _M_initialize(); }
> +  : _M_node_count(0)
> +  { }
> +#else
> +  _Rb_tree_impl() = default;
> +#endif


The default constructor of the associative containers is required to
value-initialize the comparator (see their synopses in
[map/set/multimap/multiset.overview]).

 _Rb_tree_impl() = default; doesn't do that; it default-initializes the
 comparator instead.

Tim

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[François Dumont]

[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]

On 10/10/2016 23:01, Tim Song wrote:
> Trying again...with a few edits.
>
>> On Mon, Oct 10, 2016 at 3:24 PM, François Dumont <frs.dumont@gmail.com>
>> wrote:
>>
>> @@ -602,24 +612,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>           struct _Rb_tree_impl : public _Node_allocator
>>           {
>>     _Key_compare _M_key_compare;
>> -  _Rb_tree_node_base _M_header;
>> +  _Rb_header_node _M_header;
>> +#if __cplusplus < 201103L
>>     size_type _M_node_count; // Keeps track of size of tree.
>> +#else
>> +  size_type _M_node_count = 0; // Keeps track of size of tree.
>> +#endif
>>
>> +#if __cplusplus < 201103L
>>     _Rb_tree_impl()
>> -  : _Node_allocator(), _M_key_compare(), _M_header(),
>> -    _M_node_count(0)
>> -  { _M_initialize(); }
>> +  : _M_node_count(0)
>> +  { }
>> +#else
>> +  _Rb_tree_impl() = default;
>> +#endif
>
> The default constructor of the associative containers is required to
> value-initialize the comparator (see their synopses in
> [map/set/multimap/multiset.overview]).
I don't have latest Standard version so can't see the exact word but I 
find quite annoying that the Standard doesn't allow this simple 
implementation.

I don't know if unodered containers have same kind of requirements for 
equal or hash functors but if so current implementation doesn't do this 
value initialization.

So here is another attempt. This time it simply allows to have noexcept 
condition in one place and closer to where operations are being invoked.

Ok to commit after tests ?

François
>
>   _Rb_tree_impl() = default; doesn't do that; it default-initializes the
>   comparator instead.
>
> Tim
>


[-- Attachment #2: default_dflt_const.patch --]
[-- Type: text/x-patch, Size: 5476 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h
index e5b2a1b..dea7d5b 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -167,11 +167,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      map()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      map() : _M_t() { }
+#else
+      map() = default;
+#endif
 
       /**
        *  @brief  Creates a %map with no elements.
diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h
index d240427..7e86b76 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -164,11 +164,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      multimap()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      multimap() : _M_t() { }
+#else
+      multimap() = default;
+#endif
 
       /**
        *  @brief  Creates a %multimap with no elements.
diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h
index cc068a9..7fe2fbd 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -144,11 +144,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      multiset()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      multiset() : _M_t() { }
+#else
+      multiset() = default;
+#endif
 
       /**
        *  @brief  Creates a %multiset with no elements.
diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h
index 3938351..5ed9672 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -147,11 +147,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      set()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      set() : _M_t() { }
+#else
+      set() = default;
+#endif
 
       /**
        *  @brief  Creates a %set with no elements.
diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index ee2dc70..b6a3c1e 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -137,6 +137,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
   };
 
+  struct _Rb_header_node : public _Rb_tree_node_base
+  {
+    _Rb_header_node() _GLIBCXX_NOEXCEPT
+    {
+      _M_color = _S_red;
+      _M_parent = _Base_ptr();
+      _M_left = _M_right = this;
+    }
+  };
+
   template<typename _Val>
     struct _Rb_tree_node : public _Rb_tree_node_base
     {
@@ -602,24 +612,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
         struct _Rb_tree_impl : public _Node_allocator
         {
 	  _Key_compare		_M_key_compare;
-	  _Rb_tree_node_base 	_M_header;
+	  _Rb_header_node 	_M_header;
+#if __cplusplus < 201103L
 	  size_type 		_M_node_count; // Keeps track of size of tree.
+#else
+	  size_type 		_M_node_count = 0; // Keeps track of size of tree.
+#endif
 
 	  _Rb_tree_impl()
-	  : _Node_allocator(), _M_key_compare(), _M_header(),
-	    _M_node_count(0)
-	  { _M_initialize(); }
+	  _GLIBCXX_NOEXCEPT_IF(
+	    is_nothrow_default_constructible<_Node_allocator>::value
+	    && is_nothrow_default_constructible<_Key_compare>::value)
+	  : _M_key_compare()
+#if __cplusplus < 201103L
+	  , _M_node_count(0)
+#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(); }
+	  : _Node_allocator(__a), _M_key_compare(__comp)
+#if __cplusplus < 201103L
+	  , _M_node_count(0)
+#endif
+	  { }
 
 #if __cplusplus >= 201103L
 	  _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)), _M_key_compare(__comp)
+	  { }
 #endif
 
 	  void
@@ -630,16 +650,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    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;
@@ -831,7 +841,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     public:
       // allocation/deallocation
+#if __cplusplus < 201103L
       _Rb_tree() { }
+#else
+      _Rb_tree() = default;
+#endif
 
       _Rb_tree(const _Compare& __comp,
 	       const allocator_type& __a = allocator_type())

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[Tim Song]

On Wed, Oct 12, 2016 at 4:36 PM, François Dumont <frs.dumont@gmail.com> wrote:
> On 10/10/2016 23:01, Tim Song wrote:
>>
>> Trying again...with a few edits.
>>
>>> On Mon, Oct 10, 2016 at 3:24 PM, François Dumont <frs.dumont@gmail.com>
>>> wrote:
>>>
>>> @@ -602,24 +612,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>           struct _Rb_tree_impl : public _Node_allocator
>>>           {
>>>     _Key_compare _M_key_compare;
>>> -  _Rb_tree_node_base _M_header;
>>> +  _Rb_header_node _M_header;
>>> +#if __cplusplus < 201103L
>>>     size_type _M_node_count; // Keeps track of size of tree.
>>> +#else
>>> +  size_type _M_node_count = 0; // Keeps track of size of tree.
>>> +#endif
>>>
>>> +#if __cplusplus < 201103L
>>>     _Rb_tree_impl()
>>> -  : _Node_allocator(), _M_key_compare(), _M_header(),
>>> -    _M_node_count(0)
>>> -  { _M_initialize(); }
>>> +  : _M_node_count(0)
>>> +  { }
>>> +#else
>>> +  _Rb_tree_impl() = default;
>>> +#endif
>>
>>
>> The default constructor of the associative containers is required to
>> value-initialize the comparator (see their synopses in
>> [map/set/multimap/multiset.overview]).
>
> I don't have latest Standard version so can't see the exact word but I find
> quite annoying that the Standard doesn't allow this simple implementation.

The "exact word" is:

    map() : map(Compare()) { }

which mandates the comparator be copied from a value-initialized Compare.
The use of () means value-initialization.

>
> I don't know if unodered containers have same kind of requirements for equal
> or hash functors but if so current implementation doesn't do this value
> initialization.


Yes, unordered_meows are required to do that as well. See, e.g.,
https://timsong-cpp.github.io/cppwp/unord.map.cnstr:

unordered_map() : unordered_map(size_type(see below)) { }
explicit unordered_map(size_type n,
                                      const hasher& hf = hasher(),
                                      const key_equal& eql = key_equal(),
                                      const allocator_type& a =
allocator_type());

The default constructor is specified to call the second constructor,
which copies the hasher etc. from a value-initialized object.

Tim

>
> So here is another attempt. This time it simply allows to have noexcept
> condition in one place and closer to where operations are being invoked.
>
> Ok to commit after tests ?
>
> François
>
>>
>>   _Rb_tree_impl() = default; doesn't do that; it default-initializes the
>>   comparator instead.
>>
>> Tim
>>
>

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[François Dumont]

Hi

     I have run all tests with success. Even if it doesn't get rid of 
_GLIBCXX_NOEXCEPT_IF it still limits it to one place.

     So is it ok to commit ?

François


On 12/10/2016 22:36, François Dumont wrote:
> On 10/10/2016 23:01, Tim Song wrote:
>> Trying again...with a few edits.
>>
>>> On Mon, Oct 10, 2016 at 3:24 PM, François Dumont <frs.dumont@gmail.com>
>>> wrote:
>>>
>>> @@ -602,24 +612,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>           struct _Rb_tree_impl : public _Node_allocator
>>>           {
>>>     _Key_compare _M_key_compare;
>>> -  _Rb_tree_node_base _M_header;
>>> +  _Rb_header_node _M_header;
>>> +#if __cplusplus < 201103L
>>>     size_type _M_node_count; // Keeps track of size of tree.
>>> +#else
>>> +  size_type _M_node_count = 0; // Keeps track of size of tree.
>>> +#endif
>>>
>>> +#if __cplusplus < 201103L
>>>     _Rb_tree_impl()
>>> -  : _Node_allocator(), _M_key_compare(), _M_header(),
>>> -    _M_node_count(0)
>>> -  { _M_initialize(); }
>>> +  : _M_node_count(0)
>>> +  { }
>>> +#else
>>> +  _Rb_tree_impl() = default;
>>> +#endif
>>
>> The default constructor of the associative containers is required to
>> value-initialize the comparator (see their synopses in
>> [map/set/multimap/multiset.overview]).
> I don't have latest Standard version so can't see the exact word but I 
> find quite annoying that the Standard doesn't allow this simple 
> implementation.
>
> I don't know if unodered containers have same kind of requirements for 
> equal or hash functors but if so current implementation doesn't do 
> this value initialization.
>
> So here is another attempt. This time it simply allows to have 
> noexcept condition in one place and closer to where operations are 
> being invoked.
>
> Ok to commit after tests ?
>
> François
>>
>>   _Rb_tree_impl() = default; doesn't do that; it default-initializes the
>>   comparator instead.
>>
>> Tim
>>
>

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[Jonathan Wakely]

On 12/10/16 22:36 +0200, François Dumont wrote:
>On 10/10/2016 23:01, Tim Song wrote:
>>Trying again...with a few edits.
>>
>>>On Mon, Oct 10, 2016 at 3:24 PM, François Dumont <frs.dumont@gmail.com>
>>>wrote:
>>>
>>>@@ -602,24 +612,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>          struct _Rb_tree_impl : public _Node_allocator
>>>          {
>>>    _Key_compare _M_key_compare;
>>>-  _Rb_tree_node_base _M_header;
>>>+  _Rb_header_node _M_header;
>>>+#if __cplusplus < 201103L
>>>    size_type _M_node_count; // Keeps track of size of tree.
>>>+#else
>>>+  size_type _M_node_count = 0; // Keeps track of size of tree.
>>>+#endif
>>>
>>>+#if __cplusplus < 201103L
>>>    _Rb_tree_impl()
>>>-  : _Node_allocator(), _M_key_compare(), _M_header(),
>>>-    _M_node_count(0)
>>>-  { _M_initialize(); }
>>>+  : _M_node_count(0)
>>>+  { }
>>>+#else
>>>+  _Rb_tree_impl() = default;
>>>+#endif
>>
>>The default constructor of the associative containers is required to
>>value-initialize the comparator (see their synopses in
>>[map/set/multimap/multiset.overview]).
>I don't have latest Standard version so can't see the exact word but I 
>find quite annoying that the Standard doesn't allow this simple 
>implementation.
>
>I don't know if unodered containers have same kind of requirements for 
>equal or hash functors but if so current implementation doesn't do 
>this value initialization.
>
>So here is another attempt. This time it simply allows to have 
>noexcept condition in one place and closer to where operations are 
>being invoked.
>
>Ok to commit after tests ?
>
>François
>>
>>  _Rb_tree_impl() = default; doesn't do that; it default-initializes the
>>  comparator instead.
>>
>>Tim
>>
>

>diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h
>index e5b2a1b..dea7d5b 100644
>--- a/libstdc++-v3/include/bits/stl_map.h
>+++ b/libstdc++-v3/include/bits/stl_map.h
>@@ -167,11 +167,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>       /**
>        *  @brief  Default constructor creates no elements.
>        */
>-      map()
>-      _GLIBCXX_NOEXCEPT_IF(
>-	  is_nothrow_default_constructible<allocator_type>::value
>-	  && is_nothrow_default_constructible<key_compare>::value)
>-      : _M_t() { }
>+#if __cplusplus < 201103L
>+      map() : _M_t() { }
>+#else
>+      map() = default;
>+#endif
> 
>       /**
>        *  @brief  Creates a %map with no elements.
>diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h
>index d240427..7e86b76 100644
>--- a/libstdc++-v3/include/bits/stl_multimap.h
>+++ b/libstdc++-v3/include/bits/stl_multimap.h
>@@ -164,11 +164,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>       /**
>        *  @brief  Default constructor creates no elements.
>        */
>-      multimap()
>-      _GLIBCXX_NOEXCEPT_IF(
>-	  is_nothrow_default_constructible<allocator_type>::value
>-	  && is_nothrow_default_constructible<key_compare>::value)
>-      : _M_t() { }
>+#if __cplusplus < 201103L
>+      multimap() : _M_t() { }
>+#else
>+      multimap() = default;
>+#endif
> 
>       /**
>        *  @brief  Creates a %multimap with no elements.
>diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h
>index cc068a9..7fe2fbd 100644
>--- a/libstdc++-v3/include/bits/stl_multiset.h
>+++ b/libstdc++-v3/include/bits/stl_multiset.h
>@@ -144,11 +144,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>       /**
>        *  @brief  Default constructor creates no elements.
>        */
>-      multiset()
>-      _GLIBCXX_NOEXCEPT_IF(
>-	  is_nothrow_default_constructible<allocator_type>::value
>-	  && is_nothrow_default_constructible<key_compare>::value)
>-      : _M_t() { }
>+#if __cplusplus < 201103L
>+      multiset() : _M_t() { }
>+#else
>+      multiset() = default;
>+#endif
> 
>       /**
>        *  @brief  Creates a %multiset with no elements.
>diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h
>index 3938351..5ed9672 100644
>--- a/libstdc++-v3/include/bits/stl_set.h
>+++ b/libstdc++-v3/include/bits/stl_set.h
>@@ -147,11 +147,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>       /**
>        *  @brief  Default constructor creates no elements.
>        */
>-      set()
>-      _GLIBCXX_NOEXCEPT_IF(
>-	  is_nothrow_default_constructible<allocator_type>::value
>-	  && is_nothrow_default_constructible<key_compare>::value)
>-      : _M_t() { }
>+#if __cplusplus < 201103L
>+      set() : _M_t() { }
>+#else
>+      set() = default;
>+#endif
> 
>       /**
>        *  @brief  Creates a %set with no elements.
>diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
>index ee2dc70..b6a3c1e 100644
>--- a/libstdc++-v3/include/bits/stl_tree.h
>+++ b/libstdc++-v3/include/bits/stl_tree.h
>@@ -137,6 +137,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     }
>   };
> 
>+  struct _Rb_header_node : public _Rb_tree_node_base
>+  {
>+    _Rb_header_node() _GLIBCXX_NOEXCEPT
>+    {
>+      _M_color = _S_red;
>+      _M_parent = _Base_ptr();
>+      _M_left = _M_right = this;
>+    }
>+  };
>+
>   template<typename _Val>
>     struct _Rb_tree_node : public _Rb_tree_node_base
>     {
>@@ -602,24 +612,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>         struct _Rb_tree_impl : public _Node_allocator
>         {
> 	  _Key_compare		_M_key_compare;
>-	  _Rb_tree_node_base 	_M_header;
>+	  _Rb_header_node 	_M_header;
>+#if __cplusplus < 201103L
> 	  size_type 		_M_node_count; // Keeps track of size of tree.
>+#else
>+	  size_type 		_M_node_count = 0; // Keeps track of size of tree.
>+#endif
> 
> 	  _Rb_tree_impl()
>-	  : _Node_allocator(), _M_key_compare(), _M_header(),
>-	    _M_node_count(0)
>-	  { _M_initialize(); }
>+	  _GLIBCXX_NOEXCEPT_IF(
>+	    is_nothrow_default_constructible<_Node_allocator>::value
>+	    && is_nothrow_default_constructible<_Key_compare>::value)
>+	  : _M_key_compare()
>+#if __cplusplus < 201103L
>+	  , _M_node_count(0)
>+#endif
>+	  { }

I still think this part is pointless. Why use conditional compilation
here, when we could just always set it to zero?

What is the advantage of using #if here, except adding more lines of
code?

> 
> 	  _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(); }
>+	  : _Node_allocator(__a), _M_key_compare(__comp)
>+#if __cplusplus < 201103L
>+	  , _M_node_count(0)
>+#endif
>+	  { }
> 
> #if __cplusplus >= 201103L
> 	  _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)), _M_key_compare(__comp)
>+	  { }
> #endif
> 
> 	  void
>@@ -630,16 +650,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    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;
>-	  }	    
> 	};

Since we can't remove the constructors that called _M_initialize() we
don't need to get rid of _M_initialize() either. That means we don't
need the _Rb_header_node type (so don't need to produce RTTI for a new
type).

The purpose of the patch is to allow _Rb_tree() = default and then
set() = default, map() = default etc.

That can be done by simply putting the _GLIBCXX_NOEXCEPT_IF on
_Rb_tree_impl() and forgetting all the other changes to _Rb_tree_impl.
Using a default member initializer for _M_node_count and removing
_M_initialize() aren't necessary to achieve the purpose of the patch.

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[François Dumont]

[-- Attachment #1: Type: text/plain, Size: 8641 bytes --]

On 24/10/2016 13:03, Jonathan Wakely wrote:
> On 12/10/16 22:36 +0200, François Dumont wrote:
>> On 10/10/2016 23:01, Tim Song wrote:
>>> Trying again...with a few edits.
>>>
>>>> On Mon, Oct 10, 2016 at 3:24 PM, François Dumont 
>>>> <frs.dumont@gmail.com>
>>>> wrote:
>>>>
>>>> @@ -602,24 +612,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>>          struct _Rb_tree_impl : public _Node_allocator
>>>>          {
>>>>    _Key_compare _M_key_compare;
>>>> -  _Rb_tree_node_base _M_header;
>>>> +  _Rb_header_node _M_header;
>>>> +#if __cplusplus < 201103L
>>>>    size_type _M_node_count; // Keeps track of size of tree.
>>>> +#else
>>>> +  size_type _M_node_count = 0; // Keeps track of size of tree.
>>>> +#endif
>>>>
>>>> +#if __cplusplus < 201103L
>>>>    _Rb_tree_impl()
>>>> -  : _Node_allocator(), _M_key_compare(), _M_header(),
>>>> -    _M_node_count(0)
>>>> -  { _M_initialize(); }
>>>> +  : _M_node_count(0)
>>>> +  { }
>>>> +#else
>>>> +  _Rb_tree_impl() = default;
>>>> +#endif
>>>
>>> The default constructor of the associative containers is required to
>>> value-initialize the comparator (see their synopses in
>>> [map/set/multimap/multiset.overview]).
>> I don't have latest Standard version so can't see the exact word but 
>> I find quite annoying that the Standard doesn't allow this simple 
>> implementation.
>>
>> I don't know if unodered containers have same kind of requirements 
>> for equal or hash functors but if so current implementation doesn't 
>> do this value initialization.
>>
>> So here is another attempt. This time it simply allows to have 
>> noexcept condition in one place and closer to where operations are 
>> being invoked.
>>
>> Ok to commit after tests ?
>>
>> François
>>>
>>>  _Rb_tree_impl() = default; doesn't do that; it default-initializes the
>>>  comparator instead.
>>>
>>> Tim
>>>
>>
>
>> diff --git a/libstdc++-v3/include/bits/stl_map.h 
>> b/libstdc++-v3/include/bits/stl_map.h
>> index e5b2a1b..dea7d5b 100644
>> --- a/libstdc++-v3/include/bits/stl_map.h
>> +++ b/libstdc++-v3/include/bits/stl_map.h
>> @@ -167,11 +167,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>       /**
>>        *  @brief  Default constructor creates no elements.
>>        */
>> -      map()
>> -      _GLIBCXX_NOEXCEPT_IF(
>> - is_nothrow_default_constructible<allocator_type>::value
>> -      && is_nothrow_default_constructible<key_compare>::value)
>> -      : _M_t() { }
>> +#if __cplusplus < 201103L
>> +      map() : _M_t() { }
>> +#else
>> +      map() = default;
>> +#endif
>>
>>       /**
>>        *  @brief  Creates a %map with no elements.
>> diff --git a/libstdc++-v3/include/bits/stl_multimap.h 
>> b/libstdc++-v3/include/bits/stl_multimap.h
>> index d240427..7e86b76 100644
>> --- a/libstdc++-v3/include/bits/stl_multimap.h
>> +++ b/libstdc++-v3/include/bits/stl_multimap.h
>> @@ -164,11 +164,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>       /**
>>        *  @brief  Default constructor creates no elements.
>>        */
>> -      multimap()
>> -      _GLIBCXX_NOEXCEPT_IF(
>> - is_nothrow_default_constructible<allocator_type>::value
>> -      && is_nothrow_default_constructible<key_compare>::value)
>> -      : _M_t() { }
>> +#if __cplusplus < 201103L
>> +      multimap() : _M_t() { }
>> +#else
>> +      multimap() = default;
>> +#endif
>>
>>       /**
>>        *  @brief  Creates a %multimap with no elements.
>> diff --git a/libstdc++-v3/include/bits/stl_multiset.h 
>> b/libstdc++-v3/include/bits/stl_multiset.h
>> index cc068a9..7fe2fbd 100644
>> --- a/libstdc++-v3/include/bits/stl_multiset.h
>> +++ b/libstdc++-v3/include/bits/stl_multiset.h
>> @@ -144,11 +144,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>       /**
>>        *  @brief  Default constructor creates no elements.
>>        */
>> -      multiset()
>> -      _GLIBCXX_NOEXCEPT_IF(
>> - is_nothrow_default_constructible<allocator_type>::value
>> -      && is_nothrow_default_constructible<key_compare>::value)
>> -      : _M_t() { }
>> +#if __cplusplus < 201103L
>> +      multiset() : _M_t() { }
>> +#else
>> +      multiset() = default;
>> +#endif
>>
>>       /**
>>        *  @brief  Creates a %multiset with no elements.
>> diff --git a/libstdc++-v3/include/bits/stl_set.h 
>> b/libstdc++-v3/include/bits/stl_set.h
>> index 3938351..5ed9672 100644
>> --- a/libstdc++-v3/include/bits/stl_set.h
>> +++ b/libstdc++-v3/include/bits/stl_set.h
>> @@ -147,11 +147,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>       /**
>>        *  @brief  Default constructor creates no elements.
>>        */
>> -      set()
>> -      _GLIBCXX_NOEXCEPT_IF(
>> - is_nothrow_default_constructible<allocator_type>::value
>> -      && is_nothrow_default_constructible<key_compare>::value)
>> -      : _M_t() { }
>> +#if __cplusplus < 201103L
>> +      set() : _M_t() { }
>> +#else
>> +      set() = default;
>> +#endif
>>
>>       /**
>>        *  @brief  Creates a %set with no elements.
>> diff --git a/libstdc++-v3/include/bits/stl_tree.h 
>> b/libstdc++-v3/include/bits/stl_tree.h
>> index ee2dc70..b6a3c1e 100644
>> --- a/libstdc++-v3/include/bits/stl_tree.h
>> +++ b/libstdc++-v3/include/bits/stl_tree.h
>> @@ -137,6 +137,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>     }
>>   };
>>
>> +  struct _Rb_header_node : public _Rb_tree_node_base
>> +  {
>> +    _Rb_header_node() _GLIBCXX_NOEXCEPT
>> +    {
>> +      _M_color = _S_red;
>> +      _M_parent = _Base_ptr();
>> +      _M_left = _M_right = this;
>> +    }
>> +  };
>> +
>>   template<typename _Val>
>>     struct _Rb_tree_node : public _Rb_tree_node_base
>>     {
>> @@ -602,24 +612,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>         struct _Rb_tree_impl : public _Node_allocator
>>         {
>>       _Key_compare        _M_key_compare;
>> -      _Rb_tree_node_base     _M_header;
>> +      _Rb_header_node     _M_header;
>> +#if __cplusplus < 201103L
>>       size_type         _M_node_count; // Keeps track of size of tree.
>> +#else
>> +      size_type         _M_node_count = 0; // Keeps track of size of 
>> tree.
>> +#endif
>>
>>       _Rb_tree_impl()
>> -      : _Node_allocator(), _M_key_compare(), _M_header(),
>> -        _M_node_count(0)
>> -      { _M_initialize(); }
>> +      _GLIBCXX_NOEXCEPT_IF(
>> + is_nothrow_default_constructible<_Node_allocator>::value
>> +        && is_nothrow_default_constructible<_Key_compare>::value)
>> +      : _M_key_compare()
>> +#if __cplusplus < 201103L
>> +      , _M_node_count(0)
>> +#endif
>> +      { }
>
> I still think this part is pointless. Why use conditional compilation
> here, when we could just always set it to zero?
>
> What is the advantage of using #if here, except adding more lines of
> code?
>
>>
>>       _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(); }
>> +      : _Node_allocator(__a), _M_key_compare(__comp)
>> +#if __cplusplus < 201103L
>> +      , _M_node_count(0)
>> +#endif
>> +      { }
>>
>> #if __cplusplus >= 201103L
>>       _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)), _M_key_compare(__comp)
>> +      { }
>> #endif
>>
>>       void
>> @@ -630,16 +650,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>         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;
>> -      }            };
>
> Since we can't remove the constructors that called _M_initialize() we
> don't need to get rid of _M_initialize() either. That means we don't
> need the _Rb_header_node type (so don't need to produce RTTI for a new
> type).
>
> The purpose of the patch is to allow _Rb_tree() = default and then
> set() = default, map() = default etc.
>
> That can be done by simply putting the _GLIBCXX_NOEXCEPT_IF on
> _Rb_tree_impl() and forgetting all the other changes to _Rb_tree_impl.
> Using a default member initializer for _M_node_count and removing
> _M_initialize() aren't necessary to achieve the purpose of the patch
Indeed, here is the simplified patch.

But I will come back to this base type for the next patch to generalize 
the defaulted constructors and assignment operators.

Tested under Linux x86_64, ok to commit ?

François


[-- Attachment #2: default_dflt_const.patch --]
[-- Type: text/x-patch, Size: 3692 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h
index e5b2a1b..dea7d5b 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -167,11 +167,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      map()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      map() : _M_t() { }
+#else
+      map() = default;
+#endif
 
       /**
        *  @brief  Creates a %map with no elements.
diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h
index d240427..7e86b76 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -164,11 +164,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      multimap()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      multimap() : _M_t() { }
+#else
+      multimap() = default;
+#endif
 
       /**
        *  @brief  Creates a %multimap with no elements.
diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h
index cc068a9..7fe2fbd 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -144,11 +144,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      multiset()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      multiset() : _M_t() { }
+#else
+      multiset() = default;
+#endif
 
       /**
        *  @brief  Creates a %multiset with no elements.
diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h
index 3938351..5ed9672 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -147,11 +147,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Default constructor creates no elements.
        */
-      set()
-      _GLIBCXX_NOEXCEPT_IF(
-	  is_nothrow_default_constructible<allocator_type>::value
-	  && is_nothrow_default_constructible<key_compare>::value)
-      : _M_t() { }
+#if __cplusplus < 201103L
+      set() : _M_t() { }
+#else
+      set() = default;
+#endif
 
       /**
        *  @brief  Creates a %set with no elements.
diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index ee2dc70..0bc5f4b 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -606,6 +606,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  size_type		_M_node_count; // Keeps track of size of tree.
 
 	  _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(); }
@@ -831,7 +834,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     public:
       // allocation/deallocation
+#if __cplusplus < 201103L
       _Rb_tree() { }
+#else
+      _Rb_tree() = default;
+#endif
 
       _Rb_tree(const _Compare& __comp,
 	       const allocator_type& __a = allocator_type())

Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors

[Jonathan Wakely]

On 25/10/16 21:55 +0200, François Dumont wrote:
>Indeed, here is the simplified patch.
>
>But I will come back to this base type for the next patch to 
>generalize the defaulted constructors and assignment operators.
>
>Tested under Linux x86_64, ok to commit ?

OK for trunk, thanks.