This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors
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
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())