I reviewed this patch following result of making std::pair piecewise constructor noexcept: https://gcc.gnu.org/ml/libstdc++/2018-12/msg00052.html I restore check of noexcept qualification on move constructor rather than alloc::construct cause when dealing with std::pair I want to check for noexcept qualification of Value type move constructor and it doesn't really make sens to check for alloc::construct when we eventually call alloc::construct. I'll submit this officially once back in stage 1. François On 12/4/18 10:43 PM, François Dumont wrote: > On 12/4/18 3:38 PM, Jonathan Wakely wrote: >> On 04/12/18 07:45 +0100, François Dumont wrote: >>> Hi >>> >>>   This patch fix a minor problem with usage of >>> std::move_if_noexcept. We use it to move node content if move >>> construtor is noexcept but we eventually use the >>> allocator_type::construct method which might be slightly different. >>> I think it is better to check for this method noexcept qualification. >> >> This is likely to pessimize some code, since most allocators do not >> have an exception-specification on their construct members. >> > Perhaps but the Standard mandates to call allocator construct so we > don't have choice. It is surprising to consider value_type move > constructor when we don't know what allocator construct does. > > Most users do not change from std::allocator or, even if they do, do > not implement construct so impact should be limited. > >>>   Moreover I have added a special overload for nodes containing a >>> std::pair. It is supposed to allow move semantic in associative >>> containers where Key is stored as const deleting std::pair move >>> constructor. In this case we should still move the Value part. >>> >>>   It doesn't work for the moment because the std::pair piecewise >>> constructor has no noexcept qualification. Is there any plan to add >>> it ? I think adding it will force including in stl_pair.h, >>> is it fine ? > > No feedback on this point ? Is using std::pair piecewise constructor ok ? > >>> >>>   If this evolution is accepted I'll adapt it for _Rb_tree that has >>> the same problem. >>> >>>   Working on this I also notice that content of initialization_list >>> is not moved. Is there a plan to make initialization_list iterator >>> type like move_iterator ? Should containers use >>> __make_move_iterator_if_noexcept ? >> >> No. >> >> Whether to allow moving from std::initializer_list is an active topic, >> see >> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1249r0.html >> > Ok, nice, allowing emplace build of values would be even better, I'll > have a closer look. > >>>   Tested under Linux x86_64 normal mode. >>> >>>   Ok to commit this first step ? >> >> No, this is not suitable for stage 3. It seems too risky. >> >> We can reconsider it during stage 1, but I'd like to see (at least) a >> new test showing a bug with the current code. For example, a type with >> a move constructor that is noexcept, but when used with a >> scoped_allocator_adaptor (which calls something other than the move >> constructor) we incorrectly move elements, and lose data when an >> exception happens. >> >> > Ok, I'll try. >