On 08/10/16 16:07 +0300, Ville Voutilainen wrote:
Tested on Linux-x64.
2016-10-08 Ville Voutilainen <ville.voutilai...@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 the manager when resetting the value.
This makes the state saner if an exception is thrown during the move.
(_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.
diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any
index 9160035..78bdf89 100644
--- a/libstdc++-v3/include/std/any
+++ b/libstdc++-v3/include/std/any
@@ -179,7 +179,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
typename _Tp = _Decay<_ValueType>,
typename _Mgr = _Manager<_Tp>,
__any_constructible_t<_Tp, _Args&&...> = false>
- any(in_place_type_t<_ValueType>, _Args&&... __args)
+ explicit any(in_place_type_t<_ValueType>, _Args&&... __args)
: _M_manager(&_Mgr::_S_manage)
{
_Mgr::_S_create(_M_storage, std::forward<_Args>(__args)...);
@@ -192,8 +192,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
typename _Mgr = _Manager<_Tp>,
__any_constructible_t<_Tp, initializer_list<_Up>,
_Args&&...> = false>
- any(in_place_type_t<_ValueType>,
- initializer_list<_Up> __il, _Args&&... __args)
+ explicit any(in_place_type_t<_ValueType>,
+ initializer_list<_Up> __il, _Args&&... __args)
: _M_manager(&_Mgr::_S_manage)
{
_Mgr::_S_create(_M_storage, __il, std::forward<_Args>(__args)...);
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.
@@ -211,11 +211,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
reset();
else if (this != &__rhs)
{
- if (has_value())
- _M_manager(_Op_destroy, this, nullptr);
- _Arg __arg;
- __arg._M_any = this;
- __rhs._M_manager(_Op_clone, &__rhs, &__arg);
+ 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.
}
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
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?
_Arg __arg;
__arg._M_any = this;
__rhs._M_manager(_Op_xfer, &__rhs, &__arg);
@@ -556,7 +552,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__ptr->~_Tp();
break;
case _Op_xfer:
- ::new(&__arg->_M_any->_M_storage._M_buffer) _Tp(*__ptr);
+ ::new(&__arg->_M_any->_M_storage._M_buffer) _Tp
+ (std::move(*const_cast<_Tp*>(__ptr)));
I was looking at this recently and wondering why I did a copy not a
move. *cough* no redundant operations *cough* Oops.
