From 250f5b6cc1bf4635d8c7c31fe39ee7e1a982ef07 Mon Sep 17 00:00:00 2001 From: Ville Voutilainen Date: Wed, 29 Mar 2017 02:05:21 +0300 Subject: [PATCH] Implement LWG 2900, The copy and move constructors of optional are not constexpr. Implement LWG 2900, The copy and move constructors of optional are not constexpr. * include/std/optional (_Optional_payload): New. (_Optional_base): Remove the bool parameter. (_Optional_base<_Tp, false>): Remove. (_Optional_base()): Adjust. (_Optional_base(nullopt_t)): Likewise. (_Optional_base(in_place_t, _Args&&...)): Likewise. (_Optional_base(in_place_t, initializer_list<_Up>, _Args&&...)): Likewise. (_Optional_base(const _Optional_base&)): Likewise. (_Optional_base(_Optional_base&&)): Likewise. (operator=(const _Optional_base&)): Likewise. (operator=(_Optional_base&&)): Likewise. (~_Optional_base()): Remove. (_M_is_engaged()): Adjust. (_M_get()): Likewise. (_M_construct(_Args&&...)): Likewise. (_M_destruct()): Likewise. (_M_reset()): Likewise. (_Optional_base::_Empty_byte): Remove. (_Optional_base::_M_empty): Remove. (_Optional_base::_M_payload): Adjust. * testsuite/20_util/optional/cons/value_neg.cc: Adjust. * testsuite/20_util/optional/constexpr/cons/value.cc: Add tests. From-SVN: r246556 --- libstdc++-v3/ChangeLog | 28 ++ libstdc++-v3/include/std/optional | 365 ++++++++++++--------- .../testsuite/20_util/optional/cons/value_neg.cc | 6 +- .../20_util/optional/constexpr/cons/value.cc | 17 + 4 files changed, 256 insertions(+), 160 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index b4f7e2b..a4cb6b7 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,31 @@ +2017-03-29 Ville Voutilainen + + Implement LWG 2900, The copy and move constructors + of optional are not constexpr. + * include/std/optional (_Optional_payload): New. + (_Optional_base): Remove the bool parameter. + (_Optional_base<_Tp, false>): Remove. + (_Optional_base()): Adjust. + (_Optional_base(nullopt_t)): Likewise. + (_Optional_base(in_place_t, _Args&&...)): Likewise. + (_Optional_base(in_place_t, initializer_list<_Up>, _Args&&...)): + Likewise. + (_Optional_base(const _Optional_base&)): Likewise. + (_Optional_base(_Optional_base&&)): Likewise. + (operator=(const _Optional_base&)): Likewise. + (operator=(_Optional_base&&)): Likewise. + (~_Optional_base()): Remove. + (_M_is_engaged()): Adjust. + (_M_get()): Likewise. + (_M_construct(_Args&&...)): Likewise. + (_M_destruct()): Likewise. + (_M_reset()): Likewise. + (_Optional_base::_Empty_byte): Remove. + (_Optional_base::_M_empty): Remove. + (_Optional_base::_M_payload): Adjust. + * testsuite/20_util/optional/cons/value_neg.cc: Adjust. + * testsuite/20_util/optional/constexpr/cons/value.cc: Add tests. + 2017-03-28 Jonathan Wakely PR libstdc++/80137 diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index 24802bf..1724120 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -95,178 +95,231 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __throw_bad_optional_access() { _GLIBCXX_THROW_OR_ABORT(bad_optional_access()); } - /** - * @brief Class template that holds the necessary state for @ref optional - * and that has the responsibility for construction and the special members. - * - * Such a separate base class template is necessary in order to - * conditionally enable the special members (e.g. copy/move constructors). - * Note that this means that @ref _Optional_base implements the - * functionality for copy and move assignment, but not for converting - * assignment. - * - * @see optional, _Enable_special_members - */ - template::value> - class _Optional_base - { - private: - // Remove const to avoid prohibition of reusing object storage for - // const-qualified types in [3.8/9]. This is strictly internal - // and even optional itself is oblivious to it. - using _Stored_type = remove_const_t<_Tp>; - public: + // Payload for constexpr optionals. + template ::value + && is_trivially_move_constructible<_Tp>::value, + bool /*_ShouldProvideDestructor*/ = + is_trivially_destructible<_Tp>::value> + struct _Optional_payload + { + constexpr _Optional_payload() + : _M_empty() {} - // Constructors for disengaged optionals. - constexpr _Optional_base() noexcept - : _M_empty{} { } + template + constexpr _Optional_payload(in_place_t, _Args&&... __args) + : _M_payload(std::forward<_Args>(__args)...), + _M_engaged(true) + {} - constexpr _Optional_base(nullopt_t) noexcept - : _Optional_base{} { } + 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() + {} + + 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{})) + {} - // Constructors for engaged optionals. - template, bool> = false> - constexpr explicit _Optional_base(in_place_t, _Args&&... __args) - : _M_payload(std::forward<_Args>(__args)...), _M_engaged(true) { } + using _Stored_type = remove_const_t<_Tp>; + struct _Empty_byte { }; + union { + _Empty_byte _M_empty; + _Stored_type _M_payload; + }; + bool _M_engaged = false; + }; - template&, - _Args&&...>, bool> = false> - constexpr explicit _Optional_base(in_place_t, - initializer_list<_Up> __il, - _Args&&... __args) - : _M_payload(__il, std::forward<_Args>(__args)...), - _M_engaged(true) { } + // Payload for non-constexpr optionals with non-trivial destructor. + template + struct _Optional_payload<_Tp, false, false> + { + constexpr _Optional_payload() + : _M_empty() {} - // Copy and move constructors. - _Optional_base(const _Optional_base& __other) - { - if (__other._M_engaged) - this->_M_construct(__other._M_get()); - } + template + constexpr _Optional_payload(in_place_t, _Args&&... __args) + : _M_payload(std::forward<_Args>(__args)...), + _M_engaged(true) {} - _Optional_base(_Optional_base&& __other) - noexcept(is_nothrow_move_constructible<_Tp>()) + 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(std::move(__other._M_get())); + if (__other._M_engaged) + this->_M_construct(__other._M_payload); } - // Assignment operators. - _Optional_base& - operator=(const _Optional_base& __other) + constexpr _Optional_payload(_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; + if (__other._M_engaged) + this->_M_construct(std::move(__other._M_payload)); } - _Optional_base& - operator=(_Optional_base&& __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; - // Destructor. - ~_Optional_base() + ~_Optional_payload() { - if (this->_M_engaged) - this->_M_payload.~_Stored_type(); + if (_M_engaged) + _M_payload.~_Stored_type(); } - // The following functionality is also needed by optional, hence the - // protected accessibility. - protected: - constexpr bool _M_is_engaged() const noexcept - { return this->_M_engaged; } - - // The _M_get operations have _M_engaged as a precondition. - constexpr _Tp& - _M_get() noexcept - { return _M_payload; } - - constexpr const _Tp& - _M_get() const noexcept - { return _M_payload; } - - // The _M_construct operation has !_M_engaged as a precondition - // while _M_destruct has _M_engaged as a precondition. template void _M_construct(_Args&&... __args) noexcept(is_nothrow_constructible<_Stored_type, _Args...>()) { - ::new (std::__addressof(this->_M_payload)) + ::new ((void *) std::__addressof(this->_M_payload)) _Stored_type(std::forward<_Args>(__args)...); this->_M_engaged = true; } + }; - void - _M_destruct() + // Payload for non-constexpr optionals with trivial destructor. + template + struct _Optional_payload<_Tp, false, true> + { + 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) { - this->_M_engaged = false; - this->_M_payload.~_Stored_type(); + if (__other._M_engaged) + this->_M_construct(__other._M_payload); } - // _M_reset is a 'safe' operation with no precondition. - void - _M_reset() + constexpr _Optional_payload(_Optional_payload&& __other) { - if (this->_M_engaged) - this->_M_destruct(); + if (__other._M_engaged) + this->_M_construct(std::move(__other._M_payload)); } - private: + using _Stored_type = remove_const_t<_Tp>; struct _Empty_byte { }; union { _Empty_byte _M_empty; _Stored_type _M_payload; }; bool _M_engaged = false; + + 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; + } }; - /// Partial specialization that is exactly identical to the primary template - /// save for not providing a destructor, to fulfill triviality requirements. + /** + * @brief Class template that holds the necessary state for @ref optional + * and that has the responsibility for construction and the special members. + * + * Such a separate base class template is necessary in order to + * conditionally enable the special members (e.g. copy/move constructors). + * Note that this means that @ref _Optional_base implements the + * functionality for copy and move assignment, but not for converting + * assignment. + * + * @see optional, _Enable_special_members + */ template - class _Optional_base<_Tp, false> + class _Optional_base { private: + // Remove const to avoid prohibition of reusing object storage for + // const-qualified types in [3.8/9]. This is strictly internal + // and even optional itself is oblivious to it. using _Stored_type = remove_const_t<_Tp>; public: + + // Constructors for disengaged optionals. constexpr _Optional_base() noexcept - : _M_empty{} { } + { } constexpr _Optional_base(nullopt_t) noexcept - : _Optional_base{} { } + { } + // Constructors for engaged optionals. template, bool> = false> constexpr explicit _Optional_base(in_place_t, _Args&&... __args) - : _M_payload(std::forward<_Args>(__args)...), _M_engaged(true) { } + : _M_payload(in_place, + std::forward<_Args>(__args)...) { } template __il, _Args&&... __args) - : _M_payload(__il, std::forward<_Args>(__args)...), - _M_engaged(true) { } + : _M_payload(in_place, + __il, std::forward<_Args>(__args)...) + { } - _Optional_base(const _Optional_base& __other) - { - if (__other._M_engaged) - this->_M_construct(__other._M_get()); - } + // Copy and move constructors. + constexpr _Optional_base(const _Optional_base& __other) + : _M_payload(__other._M_payload._M_engaged, + __other._M_payload) + { } - _Optional_base(_Optional_base&& __other) + constexpr _Optional_base(_Optional_base&& __other) noexcept(is_nothrow_move_constructible<_Tp>()) - { - if (__other._M_engaged) - this->_M_construct(std::move(__other._M_get())); - } + : _M_payload(__other._M_payload._M_engaged, + std::move(__other._M_payload)) + { } + // Assignment operators. _Optional_base& operator=(const _Optional_base& __other) { - if (this->_M_engaged && __other._M_engaged) - this->_M_get() = __other._M_get(); - else + if (this->_M_payload._M_engaged && __other._M_payload._M_engaged) + this->_M_get() = __other._M_get(); + else { - if (__other._M_engaged) + if (__other._M_payload._M_engaged) this->_M_construct(__other._M_get()); else this->_M_reset(); } - return *this; + + return *this; } _Optional_base& @@ -311,65 +366,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION noexcept(__and_, is_nothrow_move_assignable<_Tp>>()) { - if (this->_M_engaged && __other._M_engaged) + if (this->_M_payload._M_engaged && __other._M_payload._M_engaged) this->_M_get() = std::move(__other._M_get()); else { - if (__other._M_engaged) + if (__other._M_payload._M_engaged) this->_M_construct(std::move(__other._M_get())); else this->_M_reset(); } return *this; } - - // Sole difference - // ~_Optional_base() noexcept = default; - + // The following functionality is also needed by optional, hence the + // protected accessibility. protected: constexpr bool _M_is_engaged() const noexcept - { return this->_M_engaged; } + { return this->_M_payload._M_engaged; } + // The _M_get operations have _M_engaged as a precondition. constexpr _Tp& _M_get() noexcept - { return _M_payload; } + { return this->_M_payload._M_payload; } constexpr const _Tp& _M_get() const noexcept - { return _M_payload; } + { return this->_M_payload._M_payload; } + // The _M_construct operation has !_M_engaged as a precondition + // while _M_destruct has _M_engaged as a precondition. template void _M_construct(_Args&&... __args) noexcept(is_nothrow_constructible<_Stored_type, _Args...>()) { - ::new (std::__addressof(this->_M_payload)) + ::new (std::__addressof(this->_M_payload._M_payload)) _Stored_type(std::forward<_Args>(__args)...); - this->_M_engaged = true; + this->_M_payload._M_engaged = true; } void _M_destruct() { - this->_M_engaged = false; - this->_M_payload.~_Stored_type(); + this->_M_payload._M_engaged = false; + this->_M_payload._M_payload.~_Stored_type(); } + // _M_reset is a 'safe' operation with no precondition. void _M_reset() { - if (this->_M_engaged) + if (this->_M_payload._M_engaged) this->_M_destruct(); } private: - struct _Empty_byte { }; - union - { - _Empty_byte _M_empty; - _Stored_type _M_payload; - }; - bool _M_engaged = false; + _Optional_payload<_Tp> _M_payload; }; template 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 249f622..87907f9 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 { *-*-* } } 437 } - // { dg-error "no type" "" { target { *-*-* } } 447 } - // { dg-error "no type" "" { target { *-*-* } } 504 } + // { dg-error "no type" "" { target { *-*-* } } 488 } + // { dg-error "no type" "" { target { *-*-* } } 498 } + // { dg-error "no type" "" { target { *-*-* } } 555 } } } diff --git a/libstdc++-v3/testsuite/20_util/optional/constexpr/cons/value.cc b/libstdc++-v3/testsuite/20_util/optional/constexpr/cons/value.cc index b289f44..3b183f8 100644 --- a/libstdc++-v3/testsuite/20_util/optional/constexpr/cons/value.cc +++ b/libstdc++-v3/testsuite/20_util/optional/constexpr/cons/value.cc @@ -66,4 +66,21 @@ int main() static_assert( o, "" ); static_assert( *o == 0x1234ABCD, "" ); } + { + constexpr std::optional o = 42; + constexpr std::optional o2{o}; + constexpr std::optional o3(o); + constexpr std::optional o4 = o; + constexpr std::optional o5; + constexpr std::optional o6{o5}; + constexpr std::optional o7(o5); + constexpr std::optional o8 = o5; + constexpr std::optional o9{std::move(o)}; + constexpr std::optional o10(std::move(o)); + constexpr std::optional o11 = std::move(o); + constexpr std::optional o12; + constexpr std::optional o13{std::move(o5)}; + constexpr std::optional o14(std::move(o5)); + constexpr std::optional o15 = std::move(o5); + } } -- 2.7.4