[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