From: Ville Voutilainen Date: Mon, 15 Jan 2018 11:32:24 +0000 (+0200) Subject: Make optional conditionally trivially_{copy,move}_{constructible,assignable} X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=c89f2d24685de5663523d328814541165a0d97c3;p=gcc.git Make optional conditionally trivially_{copy,move}_{constructible,assignable} * include/std/optional (_Optional_payload): Fix the comment in the class head and turn into a primary and one specialization. (_Optional_payload::_M_engaged): Strike the NSDMI. (_Optional_payload<_Tp, false>::operator=(const _Optional_payload&)): New. (_Optional_payload<_Tp, false>::operator=(_Optional_payload&&)): Likewise. (_Optional_payload<_Tp, false>::_M_get): Likewise. (_Optional_payload<_Tp, false>::_M_reset): Likewise. (_Optional_base_impl): Likewise. (_Optional_base): Turn into a primary and three specializations. (optional(nullopt)): Change the base init. * testsuite/20_util/optional/assignment/8.cc: New. * testsuite/20_util/optional/cons/trivial.cc: Likewise. * testsuite/20_util/optional/cons/value_neg.cc: Adjust. From-SVN: r256694 --- diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index c059705cc51..92d96c46fb7 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,23 @@ +2018-01-15 Ville Voutilainen + + Make optional conditionally + trivially_{copy,move}_{constructible,assignable} + * include/std/optional (_Optional_payload): Fix the comment in + the class head and turn into a primary and one specialization. + (_Optional_payload::_M_engaged): Strike the NSDMI. + (_Optional_payload<_Tp, false>::operator=(const _Optional_payload&)): + New. + (_Optional_payload<_Tp, false>::operator=(_Optional_payload&&)): + Likewise. + (_Optional_payload<_Tp, false>::_M_get): Likewise. + (_Optional_payload<_Tp, false>::_M_reset): Likewise. + (_Optional_base_impl): Likewise. + (_Optional_base): Turn into a primary and three specializations. + (optional(nullopt)): Change the base init. + * testsuite/20_util/optional/assignment/8.cc: New. + * testsuite/20_util/optional/cons/trivial.cc: Likewise. + * testsuite/20_util/optional/cons/value_neg.cc: Adjust. + 2018-01-15 Jonathan Wakely PR libstdc++/80276 diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index 466a11c1e5f..01fc06bb724 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -100,15 +100,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Payload for constexpr optionals. template ::value - && is_trivially_move_constructible<_Tp>::value, - bool /*_ShouldProvideDestructor*/ = + bool /*_HasTrivialDestructor*/ = is_trivially_destructible<_Tp>::value> struct _Optional_payload { constexpr _Optional_payload() - : _M_empty() {} + : _M_empty(), _M_engaged(false) {} template constexpr _Optional_payload(in_place_t, _Args&&... __args) @@ -131,7 +128,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION {} constexpr _Optional_payload(__ctor_tag) - : _M_empty() + : _M_empty(), _M_engaged(false) {} constexpr _Optional_payload(__ctor_tag, _Tp&& __other) @@ -161,12 +158,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Empty_byte _M_empty; _Stored_type _M_payload; }; - bool _M_engaged = false; + bool _M_engaged; }; - // Payload for non-constexpr optionals with non-trivial destructor. + // Payload for optionals with non-trivial destructor. template - struct _Optional_payload<_Tp, false, false> + struct _Optional_payload<_Tp, false> { constexpr _Optional_payload() : _M_empty() {} @@ -203,6 +200,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION 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 { @@ -226,95 +255,87 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Stored_type(std::forward<_Args>(__args)...); this->_M_engaged = true; } - }; - // 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) + // The _M_get operations have _M_engaged as a precondition. + constexpr _Tp& + _M_get() noexcept { - if (__other._M_engaged) - this->_M_construct(__other._M_payload); + return this->_M_payload; } - constexpr _Optional_payload(_Optional_payload&& __other) + constexpr const _Tp& + _M_get() const noexcept { - if (__other._M_engaged) - this->_M_construct(std::move(__other._M_payload)); + 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(); + } + } + }; + + template + class _Optional_base_impl + { + protected: using _Stored_type = remove_const_t<_Tp>; - struct _Empty_byte { }; - union { - _Empty_byte _M_empty; - _Stored_type _M_payload; - }; - bool _M_engaged = false; - + + // 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 ((void *) std::__addressof(this->_M_payload)) - _Stored_type(std::forward<_Args>(__args)...); - this->_M_engaged = true; - } - }; + void + _M_construct(_Args&&... __args) + noexcept(is_nothrow_constructible<_Stored_type, _Args...>()) + { + ::new + (std::__addressof(static_cast<_Dp*>(this)->_M_payload._M_payload)) + _Stored_type(std::forward<_Args>(__args)...); + static_cast<_Dp*>(this)->_M_payload._M_engaged = true; + } + + void + _M_destruct() noexcept + { + static_cast<_Dp*>(this)->_M_payload._M_engaged = false; + static_cast<_Dp*>(this)->_M_payload._M_payload.~_Stored_type(); + } + + // _M_reset is a 'safe' operation with no precondition. + void + _M_reset() noexcept + { + if (static_cast<_Dp*>(this)->_M_payload._M_engaged) + static_cast<_Dp*>(this)->_M_destruct(); + } + }; /** - * @brief Class template that holds the necessary state for @ref optional - * and that has the responsibility for construction and the special members. + * @brief Class template that takes care of copy/move constructors + of optional * * 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. - * + * conditionally make copy/move constructors trivial. * @see optional, _Enable_special_members */ - template + template, + bool = is_trivially_move_constructible_v<_Tp>> class _Optional_base + // protected inheritance because optional needs to reach that base too + : protected _Optional_base_impl<_Tp, _Optional_base<_Tp>> { - 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>; - + friend class _Optional_base_impl<_Tp, _Optional_base<_Tp>>; public: // Constructors for disengaged optionals. - constexpr _Optional_base() noexcept - { } - - constexpr _Optional_base(nullopt_t) noexcept - { } + constexpr _Optional_base() = default; // Constructors for engaged optionals. template_M_payload._M_engaged; } + + // The _M_get operations have _M_engaged as a precondition. + constexpr _Tp& + _M_get() noexcept { - if (this->_M_payload._M_engaged && __other._M_payload._M_engaged) - this->_M_get() = __other._M_get(); - else - { - if (__other._M_payload._M_engaged) - this->_M_construct(__other._M_get()); - else - this->_M_reset(); - } + __glibcxx_assert(this->_M_is_engaged()); + return this->_M_payload._M_payload; + } - return *this; + constexpr const _Tp& + _M_get() const noexcept + { + __glibcxx_assert(this->_M_is_engaged()); + return this->_M_payload._M_payload; } - _Optional_base& - operator=(_Optional_base&& __other) - noexcept(__and_, - is_nothrow_move_assignable<_Tp>>()) + private: + _Optional_payload<_Tp> _M_payload; + }; + + template + class _Optional_base<_Tp, false, true> + : protected _Optional_base_impl<_Tp, _Optional_base<_Tp>> + { + friend class _Optional_base_impl<_Tp, _Optional_base<_Tp>>; + public: + + // Constructors for disengaged optionals. + constexpr _Optional_base() = default; + + // Constructors for engaged optionals. + template, bool> = false> + constexpr explicit _Optional_base(in_place_t, _Args&&... __args) + : _M_payload(in_place, + std::forward<_Args>(__args)...) { } + + template&, + _Args&&...>, bool> = false> + constexpr explicit _Optional_base(in_place_t, + initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(in_place, + __il, std::forward<_Args>(__args)...) + { } + + // Copy and move constructors. + constexpr _Optional_base(const _Optional_base& __other) + : _M_payload(__other._M_payload._M_engaged, + __other._M_payload) + { } + + constexpr _Optional_base(_Optional_base&& __other) = default; + + // Assignment operators. + _Optional_base& operator=(const _Optional_base&) = default; + _Optional_base& operator=(_Optional_base&&) = default; + + protected: + + constexpr bool _M_is_engaged() const noexcept + { return this->_M_payload._M_engaged; } + + // The _M_get operations have _M_engaged as a precondition. + constexpr _Tp& + _M_get() noexcept { - if (this->_M_payload._M_engaged && __other._M_payload._M_engaged) - this->_M_get() = std::move(__other._M_get()); - else - { - if (__other._M_payload._M_engaged) - this->_M_construct(std::move(__other._M_get())); - else - this->_M_reset(); - } - return *this; + __glibcxx_assert(this->_M_is_engaged()); + return this->_M_payload._M_payload; + } + + constexpr const _Tp& + _M_get() const noexcept + { + __glibcxx_assert(this->_M_is_engaged()); + return this->_M_payload._M_payload; } - // The following functionality is also needed by optional, hence the - // protected accessibility. + + private: + _Optional_payload<_Tp> _M_payload; + }; + + template + class _Optional_base<_Tp, true, false> + : protected _Optional_base_impl<_Tp, _Optional_base<_Tp>> + { + friend class _Optional_base_impl<_Tp, _Optional_base<_Tp>>; + public: + + // Constructors for disengaged optionals. + constexpr _Optional_base() = default; + + // Constructors for engaged optionals. + template, bool> = false> + constexpr explicit _Optional_base(in_place_t, _Args&&... __args) + : _M_payload(in_place, + std::forward<_Args>(__args)...) { } + + template&, + _Args&&...>, bool> = false> + constexpr explicit _Optional_base(in_place_t, + initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(in_place, + __il, std::forward<_Args>(__args)...) + { } + + // Copy and move constructors. + constexpr _Optional_base(const _Optional_base& __other) = default; + + constexpr _Optional_base(_Optional_base&& __other) + noexcept(is_nothrow_move_constructible<_Tp>()) + : _M_payload(__other._M_payload._M_engaged, + std::move(__other._M_payload)) + { } + + // Assignment operators. + _Optional_base& operator=(const _Optional_base&) = default; + _Optional_base& operator=(_Optional_base&&) = default; + protected: + constexpr bool _M_is_engaged() const noexcept { return this->_M_payload._M_engaged; } // The _M_get operations have _M_engaged as a precondition. constexpr _Tp& - _M_get() noexcept + _M_get() noexcept { - __glibcxx_assert(_M_is_engaged()); + __glibcxx_assert(this->_M_is_engaged()); return this->_M_payload._M_payload; } constexpr const _Tp& - _M_get() const noexcept + _M_get() const noexcept { - __glibcxx_assert(_M_is_engaged()); + __glibcxx_assert(this->_M_is_engaged()); 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._M_payload)) - _Stored_type(std::forward<_Args>(__args)...); - this->_M_payload._M_engaged = true; - } + private: + _Optional_payload<_Tp> _M_payload; + }; - void - _M_destruct() + template + class _Optional_base<_Tp, true, true> + : protected _Optional_base_impl<_Tp, _Optional_base<_Tp>> + { + friend class _Optional_base_impl<_Tp, _Optional_base<_Tp>>; + public: + + // Constructors for disengaged optionals. + constexpr _Optional_base() = default; + + // Constructors for engaged optionals. + template, bool> = false> + constexpr explicit _Optional_base(in_place_t, _Args&&... __args) + : _M_payload(in_place, + std::forward<_Args>(__args)...) { } + + template&, + _Args&&...>, bool> = false> + constexpr explicit _Optional_base(in_place_t, + initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(in_place, + __il, std::forward<_Args>(__args)...) + { } + + // Copy and move constructors. + constexpr _Optional_base(const _Optional_base& __other) = default; + constexpr _Optional_base(_Optional_base&& __other) = default; + + // Assignment operators. + _Optional_base& operator=(const _Optional_base&) = default; + _Optional_base& operator=(_Optional_base&&) = default; + + protected: + + constexpr bool _M_is_engaged() const noexcept + { return this->_M_payload._M_engaged; } + + // The _M_get operations have _M_engaged as a precondition. + constexpr _Tp& + _M_get() noexcept { - this->_M_payload._M_engaged = false; - this->_M_payload._M_payload.~_Stored_type(); + __glibcxx_assert(this->_M_is_engaged()); + return this->_M_payload._M_payload; } - // _M_reset is a 'safe' operation with no precondition. - void - _M_reset() + constexpr const _Tp& + _M_get() const noexcept { - if (this->_M_payload._M_engaged) - this->_M_destruct(); + __glibcxx_assert(this->_M_is_engaged()); + return this->_M_payload._M_payload; } private: @@ -482,8 +635,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr optional() = default; - constexpr optional(nullopt_t) noexcept - : _Base(nullopt) { } + constexpr optional(nullopt_t) noexcept { } // Converting constructors for engaged optionals. template . + +#include + +struct X +{ + ~X(); +}; + +struct Y +{ + Y& operator=(const Y&) = default; + Y& operator=(Y&&); + Y(const Y&) = default; + Y(Y&&) = default; +}; + +struct Z +{ + Z& operator=(const Z&); + Z& operator=(Z&&) = default; + Z(const Z&) = default; +}; + +struct Y2 +{ + Y2& operator=(const Y2&) = default; + Y2& operator=(Y2&&); +}; + +struct Z2 +{ + Z2& operator=(const Z2&); + Z2& operator=(Z2&&) = default; +}; + +static_assert(std::is_trivially_copy_assignable_v>); +static_assert(std::is_trivially_move_assignable_v>); +static_assert(!std::is_trivially_copy_assignable_v>); +static_assert(!std::is_trivially_move_assignable_v>); +static_assert(std::is_trivially_copy_assignable_v>); +static_assert(!std::is_trivially_move_assignable_v>); +static_assert(!std::is_trivially_copy_assignable_v>); +static_assert(std::is_trivially_move_assignable_v>); +static_assert(std::is_trivially_copy_assignable_v); +static_assert(!std::is_trivially_move_assignable_v); +static_assert(!std::is_trivially_copy_assignable_v>); +static_assert(!std::is_trivially_move_assignable_v>); +static_assert(!std::is_trivially_copy_assignable_v); +static_assert(std::is_trivially_move_assignable_v); +static_assert(!std::is_trivially_copy_assignable_v>); +static_assert(!std::is_trivially_move_assignable_v>); + + +struct S { + S(const S&&) = delete; + S& operator=(const S&) = default; +}; +static_assert(std::is_trivially_copy_assignable_v); + +union U { + char dummy; + S s; +}; +static_assert(std::is_trivially_copy_assignable_v); + +static_assert(!std::is_trivially_copy_assignable_v>); +static_assert(!std::is_copy_assignable_v>); + +struct S2 { + S2(S2&&) = delete; + S2& operator=(const S2&) = default; +}; +static_assert(std::is_trivially_move_assignable_v); + +union U2 { + char dummy; + S2 s; +}; +static_assert(std::is_trivially_move_assignable_v); + +static_assert(!std::is_trivially_move_assignable_v>); +static_assert(!std::is_move_assignable_v>); diff --git a/libstdc++-v3/testsuite/20_util/optional/cons/trivial.cc b/libstdc++-v3/testsuite/20_util/optional/cons/trivial.cc new file mode 100644 index 00000000000..84a8e101547 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/optional/cons/trivial.cc @@ -0,0 +1,47 @@ +// { dg-options "-std=gnu++17" } +// { dg-do compile } + +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +#include + +struct X +{ + ~X(); +}; + +struct Y +{ + Y(const Y&) = default; + Y(Y&&); +}; + +struct Z +{ + Z(const Z&); + Z(Z&&) = default; +}; + +static_assert(std::is_trivially_copy_constructible_v>); +static_assert(std::is_trivially_move_constructible_v>); +static_assert(!std::is_trivially_copy_constructible_v>); +static_assert(!std::is_trivially_move_constructible_v>); +static_assert(std::is_trivially_copy_constructible_v>); +static_assert(!std::is_trivially_move_constructible_v>); +static_assert(!std::is_trivially_copy_constructible_v>); +static_assert(std::is_trivially_move_constructible_v>); 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 db0cc64a885..c4489969943 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 { *-*-* } } 495 } - // { dg-error "no type" "" { target { *-*-* } } 505 } - // { dg-error "no type" "" { target { *-*-* } } 562 } + // { dg-error "no type" "" { target { *-*-* } } 647 } + // { dg-error "no type" "" { target { *-*-* } } 657 } + // { dg-error "no type" "" { target { *-*-* } } 714 } } }