Re: [v3 PATCH] Make any's copy assignment operator exception-safe, don't copy the underlying value when any is moved, make in_place constructors explicit.

[Ville Voutilainen <ville dot voutilainen at gmail dot com>]

On 10 October 2016 at 21:19, Jonathan Wakely <jwakely@redhat.com> wrote:
> I prefer to put "explicit" on a line of its own, as we do for return
> types, but I won't complain if you leave it like this.

Changed.

>> +         any(__rhs).swap(*this);
>
>
> I was trying to avoid the "redundant" xfer operations that the swap
> does, but I don't think we can do that and be exception safe. This is
> simple and safe, and I think its optimal. Thanks.

Right, as discussed, this is now just a move assignment from a temporary.

>
>>         }
>>       return *this;
>>     }
>> @@ -232,7 +228,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>       else if (this != &__rhs)
>>         {
>>           if (has_value())
>> -           _M_manager(_Op_destroy, this, nullptr);
>> +           reset();
>
>
> If you're going to use reset() then you don't need the has_value()
> check first. I think the reason I didn't use reset() was to avoid the

I removed the check, works fine.

> dead store to _M_manager that reset() does, since the compiler might
> not detect it's dead (because the next store is done by the call
> through a function pointer).
> This code was all pretty carefully written to avoid any redundant
> operations. Does this change buy us anything except simpler code?

As discussed, destroying the value but leaving the manager non-null will
do bad things.

New patch attached, ok for trunk?

2016-10-10  Ville Voutilainen  <ville.voutilainen@gmail.com>

    Make any's copy assignment operator exception-safe,
    don't copy the underlying value when any is moved,
    make in_place constructors explicit.
    * include/std/any (any(in_place_type_t<_ValueType>, _Args&&...)):
    Make explicit.
    (any(in_place_type_t<_ValueType>, initializer_list<_Up>, _Args&&...)):
    Likewise.
    (operator=(const any&)): Make strongly exception-safe.
    (operator=(any&&)): reset() unconditionally in the case where
    rhs has a value.
    (operator=(_ValueType&&)): Indent the return type.
    (_Manager_internal<_Tp>::_S_manage): Move in _Op_xfer, don't copy.
    * testsuite/20_util/any/assign/2.cc: Adjust.
    * testsuite/20_util/any/assign/exception.cc: New.
    * testsuite/20_util/any/cons/2.cc: Adjust.
    * testsuite/20_util/any/cons/explicit.cc: New.
    * testsuite/20_util/any/misc/any_cast_neg.cc: Ajust.

Attachment: any-fixes_2.diff
Description: Text document