From: Ville Voutilainen Date: Tue, 6 Mar 2018 21:43:03 +0000 (+0200) Subject: re PR libstdc++/84601 (std::optional> is not assignment copyable) X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=447346e465c50ad6d840c2c29c9a07417e8d219c;p=gcc.git re PR libstdc++/84601 (std::optional> is not assignment copyable) PR libstdc++/84601 * include/std/optional (_Optional_payload): Split into multiple specializations that can handle different cases of trivial or non-trivial assignment operators. * testsuite/20_util/optional/84601.cc: New. * testsuite/20_util/optional/cons/value_neg.cc: Adjust. From-SVN: r258304 --- diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index e38def1b850..b0b3a956ace 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,12 @@ +2018-03-06 Ville Voutilainen + + PR libstdc++/84601 + * include/std/optional (_Optional_payload): Split into multiple + specializations that can handle different cases of trivial or + non-trivial assignment operators. + * testsuite/20_util/optional/84601.cc: New. + * testsuite/20_util/optional/cons/value_neg.cc: Adjust. + 2018-03-02 Jonathan Wakely PR libstdc++/84671 diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index 01fc06bb724..0aa20dd9437 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -98,11 +98,135 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { _GLIBCXX_THROW_OR_ABORT(bad_optional_access()); } - // Payload for constexpr optionals. + // Payload for optionals with non-trivial destructor. template ::value> + is_trivially_destructible<_Tp>::value, + bool /*_HasTrivialCopyAssignment*/ = + is_trivially_copy_assignable<_Tp>::value, + bool /*_HasTrivialMoveAssignment*/ = + is_trivially_move_assignable<_Tp>::value> struct _Optional_payload + { + constexpr _Optional_payload() + : _M_empty() {} + + template + constexpr _Optional_payload(in_place_t, _Args&&... __args) + : _M_payload(std::forward<_Args>(__args)...), + _M_engaged(true) {} + + template + constexpr _Optional_payload(std::initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(__il, std::forward<_Args>(__args)...), + _M_engaged(true) {} + constexpr + _Optional_payload(bool __engaged, const _Optional_payload& __other) + : _Optional_payload(__other) + {} + + constexpr + _Optional_payload(bool __engaged, _Optional_payload&& __other) + : _Optional_payload(std::move(__other)) + {} + + constexpr _Optional_payload(const _Optional_payload& __other) + { + if (__other._M_engaged) + this->_M_construct(__other._M_payload); + } + + constexpr _Optional_payload(_Optional_payload&& __other) + { + if (__other._M_engaged) + this->_M_construct(std::move(__other._M_payload)); + } + + _Optional_payload& + operator=(const _Optional_payload& __other) + { + if (this->_M_engaged && __other._M_engaged) + this->_M_get() = __other._M_get(); + else + { + if (__other._M_engaged) + this->_M_construct(__other._M_get()); + else + this->_M_reset(); + } + return *this; + } + + _Optional_payload& + operator=(_Optional_payload&& __other) + noexcept(__and_, + is_nothrow_move_assignable<_Tp>>()) + { + if (this->_M_engaged && __other._M_engaged) + this->_M_get() = std::move(__other._M_get()); + else + { + if (__other._M_engaged) + this->_M_construct(std::move(__other._M_get())); + else + this->_M_reset(); + } + return *this; + } + + using _Stored_type = remove_const_t<_Tp>; + struct _Empty_byte { }; + union { + _Empty_byte _M_empty; + _Stored_type _M_payload; + }; + bool _M_engaged = false; + + ~_Optional_payload() + { + if (_M_engaged) + _M_payload.~_Stored_type(); + } + + template + void + _M_construct(_Args&&... __args) + noexcept(is_nothrow_constructible<_Stored_type, _Args...>()) + { + ::new ((void *) std::__addressof(this->_M_payload)) + _Stored_type(std::forward<_Args>(__args)...); + this->_M_engaged = true; + } + + // The _M_get operations have _M_engaged as a precondition. + constexpr _Tp& + _M_get() noexcept + { + return this->_M_payload; + } + + constexpr const _Tp& + _M_get() const noexcept + { + return this->_M_payload; + } + + // _M_reset is a 'safe' operation with no precondition. + void + _M_reset() noexcept + { + if (this->_M_engaged) + { + this->_M_engaged = false; + this->_M_payload.~_Stored_type(); + } + } + }; + + // Payload for constexpr optionals. + template + struct _Optional_payload<_Tp, true, true, true> { constexpr _Optional_payload() : _M_empty(), _M_engaged(false) {} @@ -161,44 +285,294 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION bool _M_engaged; }; - // Payload for optionals with non-trivial destructor. + // Payload for optionals with non-trivial copy assignment. template - struct _Optional_payload<_Tp, false> + struct _Optional_payload<_Tp, true, false, true> { constexpr _Optional_payload() - : _M_empty() {} + : _M_empty(), _M_engaged(false) {} - template + template constexpr _Optional_payload(in_place_t, _Args&&... __args) : _M_payload(std::forward<_Args>(__args)...), + _M_engaged(true) + {} + + template + constexpr _Optional_payload(std::initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(__il, std::forward<_Args>(__args)...), _M_engaged(true) {} + template struct __ctor_tag {}; + + constexpr _Optional_payload(__ctor_tag, + const _Tp& __other) + : _M_payload(__other), + _M_engaged(true) + {} + + constexpr _Optional_payload(__ctor_tag) + : _M_empty(), _M_engaged(false) + {} + + constexpr _Optional_payload(__ctor_tag, _Tp&& __other) + : _M_payload(std::move(__other)), + _M_engaged(true) + {} + + constexpr _Optional_payload(bool __engaged, + const _Optional_payload& __other) + : _Optional_payload(__engaged ? + _Optional_payload(__ctor_tag{}, + __other._M_payload) : + _Optional_payload(__ctor_tag{})) + {} + + constexpr _Optional_payload(bool __engaged, + _Optional_payload&& __other) + : _Optional_payload(__engaged + ? _Optional_payload(__ctor_tag{}, + std::move(__other._M_payload)) + : _Optional_payload(__ctor_tag{})) + {} + + _Optional_payload(const _Optional_payload&) = default; + _Optional_payload(_Optional_payload&&) = default; + + _Optional_payload& + operator=(const _Optional_payload& __other) + { + if (this->_M_engaged && __other._M_engaged) + this->_M_get() = __other._M_get(); + else + { + if (__other._M_engaged) + this->_M_construct(__other._M_get()); + else + this->_M_reset(); + } + return *this; + } + + _Optional_payload& + operator=(_Optional_payload&& __other) = default; + + using _Stored_type = remove_const_t<_Tp>; + struct _Empty_byte { }; + union { + _Empty_byte _M_empty; + _Stored_type _M_payload; + }; + bool _M_engaged; + + template + void + _M_construct(_Args&&... __args) + noexcept(is_nothrow_constructible<_Stored_type, _Args...>()) + { + ::new ((void *) std::__addressof(this->_M_payload)) + _Stored_type(std::forward<_Args>(__args)...); + this->_M_engaged = true; + } + + // The _M_get operations have _M_engaged as a precondition. + constexpr _Tp& + _M_get() noexcept + { + return this->_M_payload; + } + + constexpr const _Tp& + _M_get() const noexcept + { + return this->_M_payload; + } + + // _M_reset is a 'safe' operation with no precondition. + void + _M_reset() noexcept + { + if (this->_M_engaged) + { + this->_M_engaged = false; + this->_M_payload.~_Stored_type(); + } + } + }; + + // Payload for optionals with non-trivial move assignment. + template + struct _Optional_payload<_Tp, true, true, false> + { + constexpr _Optional_payload() + : _M_empty(), _M_engaged(false) {} + + template + constexpr _Optional_payload(in_place_t, _Args&&... __args) + : _M_payload(std::forward<_Args>(__args)...), + _M_engaged(true) + {} + template constexpr _Optional_payload(std::initializer_list<_Up> __il, _Args&&... __args) : _M_payload(__il, std::forward<_Args>(__args)...), _M_engaged(true) {} - constexpr - _Optional_payload(bool __engaged, const _Optional_payload& __other) - : _Optional_payload(__other) + + template struct __ctor_tag {}; + + constexpr _Optional_payload(__ctor_tag, + const _Tp& __other) + : _M_payload(__other), + _M_engaged(true) {} - constexpr - _Optional_payload(bool __engaged, _Optional_payload&& __other) - : _Optional_payload(std::move(__other)) + constexpr _Optional_payload(__ctor_tag) + : _M_empty(), _M_engaged(false) {} - constexpr _Optional_payload(const _Optional_payload& __other) + constexpr _Optional_payload(__ctor_tag, _Tp&& __other) + : _M_payload(std::move(__other)), + _M_engaged(true) + {} + + constexpr _Optional_payload(bool __engaged, + const _Optional_payload& __other) + : _Optional_payload(__engaged ? + _Optional_payload(__ctor_tag{}, + __other._M_payload) : + _Optional_payload(__ctor_tag{})) + {} + + constexpr _Optional_payload(bool __engaged, + _Optional_payload&& __other) + : _Optional_payload(__engaged + ? _Optional_payload(__ctor_tag{}, + std::move(__other._M_payload)) + : _Optional_payload(__ctor_tag{})) + {} + + _Optional_payload(const _Optional_payload&) = default; + _Optional_payload(_Optional_payload&&) = default; + + _Optional_payload& + operator=(const _Optional_payload& __other) = default; + + _Optional_payload& + operator=(_Optional_payload&& __other) + noexcept(__and_, + is_nothrow_move_assignable<_Tp>>()) { - if (__other._M_engaged) - this->_M_construct(__other._M_payload); + if (this->_M_engaged && __other._M_engaged) + this->_M_get() = std::move(__other._M_get()); + else + { + if (__other._M_engaged) + this->_M_construct(std::move(__other._M_get())); + else + this->_M_reset(); + } + return *this; } - constexpr _Optional_payload(_Optional_payload&& __other) + using _Stored_type = remove_const_t<_Tp>; + struct _Empty_byte { }; + union { + _Empty_byte _M_empty; + _Stored_type _M_payload; + }; + bool _M_engaged; + + template + void + _M_construct(_Args&&... __args) + noexcept(is_nothrow_constructible<_Stored_type, _Args...>()) + { + ::new ((void *) std::__addressof(this->_M_payload)) + _Stored_type(std::forward<_Args>(__args)...); + this->_M_engaged = true; + } + + // The _M_get operations have _M_engaged as a precondition. + constexpr _Tp& + _M_get() noexcept { - if (__other._M_engaged) - this->_M_construct(std::move(__other._M_payload)); + return this->_M_payload; + } + + constexpr const _Tp& + _M_get() const noexcept + { + return this->_M_payload; + } + + // _M_reset is a 'safe' operation with no precondition. + void + _M_reset() noexcept + { + if (this->_M_engaged) + { + this->_M_engaged = false; + this->_M_payload.~_Stored_type(); + } } + }; + + // Payload for optionals with non-trivial copy and move assignment. + template + struct _Optional_payload<_Tp, true, false, false> + { + constexpr _Optional_payload() + : _M_empty(), _M_engaged(false) {} + + template + constexpr _Optional_payload(in_place_t, _Args&&... __args) + : _M_payload(std::forward<_Args>(__args)...), + _M_engaged(true) + {} + + template + constexpr _Optional_payload(std::initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(__il, std::forward<_Args>(__args)...), + _M_engaged(true) {} + + template struct __ctor_tag {}; + + constexpr _Optional_payload(__ctor_tag, + const _Tp& __other) + : _M_payload(__other), + _M_engaged(true) + {} + + constexpr _Optional_payload(__ctor_tag) + : _M_empty(), _M_engaged(false) + {} + + constexpr _Optional_payload(__ctor_tag, _Tp&& __other) + : _M_payload(std::move(__other)), + _M_engaged(true) + {} + + constexpr _Optional_payload(bool __engaged, + const _Optional_payload& __other) + : _Optional_payload(__engaged ? + _Optional_payload(__ctor_tag{}, + __other._M_payload) : + _Optional_payload(__ctor_tag{})) + {} + + constexpr _Optional_payload(bool __engaged, + _Optional_payload&& __other) + : _Optional_payload(__engaged + ? _Optional_payload(__ctor_tag{}, + std::move(__other._M_payload)) + : _Optional_payload(__ctor_tag{})) + {} + + _Optional_payload(const _Optional_payload&) = default; + _Optional_payload(_Optional_payload&&) = default; _Optional_payload& operator=(const _Optional_payload& __other) @@ -238,13 +612,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Empty_byte _M_empty; _Stored_type _M_payload; }; - bool _M_engaged = false; - - ~_Optional_payload() - { - if (_M_engaged) - _M_payload.~_Stored_type(); - } + bool _M_engaged; template void @@ -280,7 +648,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } }; - + template class _Optional_base_impl { diff --git a/libstdc++-v3/testsuite/20_util/optional/84601.cc b/libstdc++-v3/testsuite/20_util/optional/84601.cc new file mode 100644 index 00000000000..e86d39e277d --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/optional/84601.cc @@ -0,0 +1,22 @@ +// { dg-options "-std=gnu++17" } +// { dg-do compile } + +#include + +using pair_t = std::pair; +using opt_t = std::optional; + +static_assert(std::is_copy_constructible_v); +static_assert(std::is_copy_assignable_v); + +static_assert(std::is_copy_assignable_v); // assertion fails. + +class A +{ + void f(const opt_t& opt) + { + _opt = opt; + } + + opt_t _opt; +}; diff --git a/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc b/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc index c4489969943..ae55ab233f1 100644 --- a/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc +++ b/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc @@ -37,8 +37,8 @@ int main() std::optional> oup2 = new int; // { dg-error "conversion" } struct U { explicit U(std::in_place_t); }; std::optional ou(std::in_place); // { dg-error "no matching" } - // { dg-error "no type" "" { target { *-*-* } } 647 } - // { dg-error "no type" "" { target { *-*-* } } 657 } - // { dg-error "no type" "" { target { *-*-* } } 714 } + // { dg-error "no type" "" { target { *-*-* } } 1015 } + // { dg-error "no type" "" { target { *-*-* } } 1025 } + // { dg-error "no type" "" { target { *-*-* } } 1082 } } }