This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]